Reorganize pep508 marker module (#5433)

## Summary

Just moves some code around into separate modules, the current file is a
bit too large to navigate.

crates/pep508-rs/src/lib.rs

@@ -1043,7 +1043,7 @@ fn parse_pep508_requirement<T: Pep508Url>(
     let marker = if cursor.peek_char() == Some(';') {
         // Skip past the semicolon
         cursor.next();
-        Some(marker::parse_markers_cursor(cursor, reporter)?)
+        Some(marker::parse::parse_markers_cursor(cursor, reporter)?)
     } else {
         None
     };
@@ -1124,8 +1124,7 @@ mod tests {
 
     use crate::cursor::Cursor;
     use crate::marker::{
-        parse_markers_cursor, MarkerExpression, MarkerOperator, MarkerTree, MarkerValueString,
-        MarkerValueVersion,
+        parse, MarkerExpression, MarkerOperator, MarkerTree, MarkerValueString, MarkerValueVersion,
     };
     use crate::{Requirement, TracingReporter, VerbatimUrl, VersionOrUrl};
 
@@ -1460,9 +1459,11 @@ mod tests {
     #[test]
     fn test_marker_parsing() {
         let marker = r#"python_version == "2.7" and (sys_platform == "win32" or (os_name == "linux" and implementation_name == 'cpython'))"#;
-        let actual =
-            parse_markers_cursor::<VerbatimUrl>(&mut Cursor::new(marker), &mut TracingReporter)
-                .unwrap();
+        let actual = parse::parse_markers_cursor::<VerbatimUrl>(
+            &mut Cursor::new(marker),
+            &mut TracingReporter,
+        )
+        .unwrap();
         let expected = MarkerTree::And(vec![
             MarkerTree::Expression(MarkerExpression::Version {
                 key: MarkerValueVersion::PythonVersion,

crates/pep508-rs/src/marker/environment.rs

@@ -0,0 +1,562 @@
+use std::str::FromStr;
+use std::sync::Arc;
+
+use pep440_rs::{Version, VersionParseError};
+#[cfg(feature = "pyo3")]
+use pyo3::{exceptions::PyValueError, pyclass, pymethods, types::PyAnyMethods, PyResult, Python};
+
+use crate::{MarkerValueString, MarkerValueVersion, StringVersion};
+
+/// The marker values for a python interpreter, normally the current one
+///
+/// <https://packaging.python.org/en/latest/specifications/dependency-specifiers/#environment-markers>
+///
+/// Some are `(String, Version)` because we have to support version comparison
+#[allow(missing_docs, clippy::unsafe_derive_deserialize)]
+#[derive(Clone, Debug, Eq, Hash, PartialEq, serde::Deserialize, serde::Serialize)]
+#[cfg_attr(feature = "pyo3", pyclass(module = "pep508"))]
+pub struct MarkerEnvironment {
+    #[serde(flatten)]
+    inner: Arc<MarkerEnvironmentInner>,
+}
+
+#[derive(Clone, Debug, Eq, Hash, PartialEq, serde::Deserialize, serde::Serialize)]
+struct MarkerEnvironmentInner {
+    implementation_name: String,
+    implementation_version: StringVersion,
+    os_name: String,
+    platform_machine: String,
+    platform_python_implementation: String,
+    platform_release: String,
+    platform_system: String,
+    platform_version: String,
+    python_full_version: StringVersion,
+    python_version: StringVersion,
+    sys_platform: String,
+}
+
+impl MarkerEnvironment {
+    /// Returns of the PEP 440 version typed value of the key in the current environment
+    pub fn get_version(&self, key: &MarkerValueVersion) -> &Version {
+        match key {
+            MarkerValueVersion::ImplementationVersion => &self.implementation_version().version,
+            MarkerValueVersion::PythonFullVersion => &self.python_full_version().version,
+            MarkerValueVersion::PythonVersion => &self.python_version().version,
+        }
+    }
+
+    /// Returns of the stringly typed value of the key in the current environment
+    pub fn get_string(&self, key: &MarkerValueString) -> &str {
+        match key {
+            MarkerValueString::ImplementationName => self.implementation_name(),
+            MarkerValueString::OsName | MarkerValueString::OsNameDeprecated => self.os_name(),
+            MarkerValueString::PlatformMachine | MarkerValueString::PlatformMachineDeprecated => {
+                self.platform_machine()
+            }
+            MarkerValueString::PlatformPythonImplementation
+            | MarkerValueString::PlatformPythonImplementationDeprecated
+            | MarkerValueString::PythonImplementationDeprecated => {
+                self.platform_python_implementation()
+            }
+            MarkerValueString::PlatformRelease => self.platform_release(),
+            MarkerValueString::PlatformSystem => self.platform_system(),
+            MarkerValueString::PlatformVersion | MarkerValueString::PlatformVersionDeprecated => {
+                self.platform_version()
+            }
+            MarkerValueString::SysPlatform | MarkerValueString::SysPlatformDeprecated => {
+                self.sys_platform()
+            }
+        }
+    }
+}
+
+/// APIs for retrieving specific parts of a marker environment.
+impl MarkerEnvironment {
+    /// Returns the name of the Python implementation for this environment.
+    ///
+    /// This is equivalent to `sys.implementation.name`.
+    ///
+    /// Some example values are: `cpython`.
+    #[inline]
+    pub fn implementation_name(&self) -> &str {
+        &self.inner.implementation_name
+    }
+
+    /// Returns the Python implementation version for this environment.
+    ///
+    /// This value is derived from `sys.implementation.version`. See [PEP 508
+    /// environment markers] for full details.
+    ///
+    /// This is equivalent to `sys.implementation.name`.
+    ///
+    /// Some example values are: `3.4.0`, `3.5.0b1`.
+    ///
+    /// [PEP 508 environment markers]: https://peps.python.org/pep-0508/#environment-markers
+    #[inline]
+    pub fn implementation_version(&self) -> &StringVersion {
+        &self.inner.implementation_version
+    }
+
+    /// Returns the name of the operating system for this environment.
+    ///
+    /// This is equivalent to `os.name`.
+    ///
+    /// Some example values are: `posix`, `java`.
+    #[inline]
+    pub fn os_name(&self) -> &str {
+        &self.inner.os_name
+    }
+
+    /// Returns the name of the machine for this environment's platform.
+    ///
+    /// This is equivalent to `platform.machine()`.
+    ///
+    /// Some example values are: `x86_64`.
+    #[inline]
+    pub fn platform_machine(&self) -> &str {
+        &self.inner.platform_machine
+    }
+
+    /// Returns the name of the Python implementation for this environment's
+    /// platform.
+    ///
+    /// This is equivalent to `platform.python_implementation()`.
+    ///
+    /// Some example values are: `CPython`, `Jython`.
+    #[inline]
+    pub fn platform_python_implementation(&self) -> &str {
+        &self.inner.platform_python_implementation
+    }
+
+    /// Returns the release for this environment's platform.
+    ///
+    /// This is equivalent to `platform.release()`.
+    ///
+    /// Some example values are: `3.14.1-x86_64-linode39`, `14.5.0`, `1.8.0_51`.
+    #[inline]
+    pub fn platform_release(&self) -> &str {
+        &self.inner.platform_release
+    }
+
+    /// Returns the system for this environment's platform.
+    ///
+    /// This is equivalent to `platform.system()`.
+    ///
+    /// Some example values are: `Linux`, `Windows`, `Java`.
+    #[inline]
+    pub fn platform_system(&self) -> &str {
+        &self.inner.platform_system
+    }
+
+    /// Returns the version for this environment's platform.
+    ///
+    /// This is equivalent to `platform.version()`.
+    ///
+    /// Some example values are: `#1 SMP Fri Apr 25 13:07:35 EDT 2014`,
+    /// `Java HotSpot(TM) 64-Bit Server VM, 25.51-b03, Oracle Corporation`,
+    /// `Darwin Kernel Version 14.5.0: Wed Jul 29 02:18:53 PDT 2015;
+    /// root:xnu-2782.40.9~2/RELEASE_X86_64`.
+    #[inline]
+    pub fn platform_version(&self) -> &str {
+        &self.inner.platform_version
+    }
+
+    /// Returns the full version of Python for this environment.
+    ///
+    /// This is equivalent to `platform.python_version()`.
+    ///
+    /// Some example values are: `3.4.0`, `3.5.0b1`.
+    #[inline]
+    pub fn python_full_version(&self) -> &StringVersion {
+        &self.inner.python_full_version
+    }
+
+    /// Returns the version of Python for this environment.
+    ///
+    /// This is equivalent to `'.'.join(platform.python_version_tuple()[:2])`.
+    ///
+    /// Some example values are: `3.4`, `2.7`.
+    #[inline]
+    pub fn python_version(&self) -> &StringVersion {
+        &self.inner.python_version
+    }
+
+    /// Returns the name of the system platform for this environment.
+    ///
+    /// This is equivalent to `sys.platform`.
+    ///
+    /// Some example values are: `linux`, `linux2`, `darwin`, `java1.8.0_51`
+    /// (note that `linux` is from Python3 and `linux2` from Python2).
+    #[inline]
+    pub fn sys_platform(&self) -> &str {
+        &self.inner.sys_platform
+    }
+}
+
+/// APIs for setting specific parts of a marker environment.
+impl MarkerEnvironment {
+    /// Set the name of the Python implementation for this environment.
+    ///
+    /// See also [`MarkerEnvironment::implementation_name`].
+    #[inline]
+    #[must_use]
+    pub fn with_implementation_name(mut self, value: impl Into<String>) -> MarkerEnvironment {
+        Arc::make_mut(&mut self.inner).implementation_name = value.into();
+        self
+    }
+
+    /// Set the Python implementation version for this environment.
+    ///
+    /// See also [`MarkerEnvironment::implementation_version`].
+    #[inline]
+    #[must_use]
+    pub fn with_implementation_version(
+        mut self,
+        value: impl Into<StringVersion>,
+    ) -> MarkerEnvironment {
+        Arc::make_mut(&mut self.inner).implementation_version = value.into();
+        self
+    }
+
+    /// Set the name of the operating system for this environment.
+    ///
+    /// See also [`MarkerEnvironment::os_name`].
+    #[inline]
+    #[must_use]
+    pub fn with_os_name(mut self, value: impl Into<String>) -> MarkerEnvironment {
+        Arc::make_mut(&mut self.inner).os_name = value.into();
+        self
+    }
+
+    /// Set the name of the machine for this environment's platform.
+    ///
+    /// See also [`MarkerEnvironment::platform_machine`].
+    #[inline]
+    #[must_use]
+    pub fn with_platform_machine(mut self, value: impl Into<String>) -> MarkerEnvironment {
+        Arc::make_mut(&mut self.inner).platform_machine = value.into();
+        self
+    }
+
+    /// Set the name of the Python implementation for this environment's
+    /// platform.
+    ///
+    /// See also [`MarkerEnvironment::platform_python_implementation`].
+    #[inline]
+    #[must_use]
+    pub fn with_platform_python_implementation(
+        mut self,
+        value: impl Into<String>,
+    ) -> MarkerEnvironment {
+        Arc::make_mut(&mut self.inner).platform_python_implementation = value.into();
+        self
+    }
+
+    /// Set the release for this environment's platform.
+    ///
+    /// See also [`MarkerEnvironment::platform_release`].
+    #[inline]
+    #[must_use]
+    pub fn with_platform_release(mut self, value: impl Into<String>) -> MarkerEnvironment {
+        Arc::make_mut(&mut self.inner).platform_release = value.into();
+        self
+    }
+
+    /// Set the system for this environment's platform.
+    ///
+    /// See also [`MarkerEnvironment::platform_system`].
+    #[inline]
+    #[must_use]
+    pub fn with_platform_system(mut self, value: impl Into<String>) -> MarkerEnvironment {
+        Arc::make_mut(&mut self.inner).platform_system = value.into();
+        self
+    }
+
+    /// Set the version for this environment's platform.
+    ///
+    /// See also [`MarkerEnvironment::platform_version`].
+    #[inline]
+    #[must_use]
+    pub fn with_platform_version(mut self, value: impl Into<String>) -> MarkerEnvironment {
+        Arc::make_mut(&mut self.inner).platform_version = value.into();
+        self
+    }
+
+    /// Set the full version of Python for this environment.
+    ///
+    /// See also [`MarkerEnvironment::python_full_version`].
+    #[inline]
+    #[must_use]
+    pub fn with_python_full_version(
+        mut self,
+        value: impl Into<StringVersion>,
+    ) -> MarkerEnvironment {
+        Arc::make_mut(&mut self.inner).python_full_version = value.into();
+        self
+    }
+
+    /// Set the version of Python for this environment.
+    ///
+    /// See also [`MarkerEnvironment::python_full_version`].
+    #[inline]
+    #[must_use]
+    pub fn with_python_version(mut self, value: impl Into<StringVersion>) -> MarkerEnvironment {
+        Arc::make_mut(&mut self.inner).python_version = value.into();
+        self
+    }
+
+    /// Set the name of the system platform for this environment.
+    ///
+    /// See also [`MarkerEnvironment::sys_platform`].
+    #[inline]
+    #[must_use]
+    pub fn with_sys_platform(mut self, value: impl Into<String>) -> MarkerEnvironment {
+        Arc::make_mut(&mut self.inner).sys_platform = value.into();
+        self
+    }
+}
+
+#[cfg(feature = "pyo3")]
+#[pymethods]
+impl MarkerEnvironment {
+    /// Construct your own marker environment
+    #[new]
+    #[pyo3(signature = (*,
+        implementation_name,
+        implementation_version,
+        os_name,
+        platform_machine,
+        platform_python_implementation,
+        platform_release,
+        platform_system,
+        platform_version,
+        python_full_version,
+        python_version,
+        sys_platform
+    ))]
+    fn py_new(
+        implementation_name: &str,
+        implementation_version: &str,
+        os_name: &str,
+        platform_machine: &str,
+        platform_python_implementation: &str,
+        platform_release: &str,
+        platform_system: &str,
+        platform_version: &str,
+        python_full_version: &str,
+        python_version: &str,
+        sys_platform: &str,
+    ) -> PyResult<Self> {
+        let implementation_version =
+            StringVersion::from_str(implementation_version).map_err(|err| {
+                PyValueError::new_err(format!(
+                    "implementation_version is not a valid PEP440 version: {err}"
+                ))
+            })?;
+        let python_full_version = StringVersion::from_str(python_full_version).map_err(|err| {
+            PyValueError::new_err(format!(
+                "python_full_version is not a valid PEP440 version: {err}"
+            ))
+        })?;
+        let python_version = StringVersion::from_str(python_version).map_err(|err| {
+            PyValueError::new_err(format!(
+                "python_version is not a valid PEP440 version: {err}"
+            ))
+        })?;
+        Ok(Self {
+            inner: Arc::new(MarkerEnvironmentInner {
+                implementation_name: implementation_name.to_string(),
+                implementation_version,
+                os_name: os_name.to_string(),
+                platform_machine: platform_machine.to_string(),
+                platform_python_implementation: platform_python_implementation.to_string(),
+                platform_release: platform_release.to_string(),
+                platform_system: platform_system.to_string(),
+                platform_version: platform_version.to_string(),
+                python_full_version,
+                python_version,
+                sys_platform: sys_platform.to_string(),
+            }),
+        })
+    }
+
+    /// Query the current python interpreter to get the correct marker value
+    #[staticmethod]
+    fn current(py: Python<'_>) -> PyResult<Self> {
+        let os = py.import_bound("os")?;
+        let platform = py.import_bound("platform")?;
+        let sys = py.import_bound("sys")?;
+        let python_version_tuple: (String, String, String) = platform
+            .getattr("python_version_tuple")?
+            .call0()?
+            .extract()?;
+
+        // See pseudocode at
+        // https://packaging.python.org/en/latest/specifications/dependency-specifiers/#environment-markers
+        let name = sys.getattr("implementation")?.getattr("name")?.extract()?;
+        let info = sys.getattr("implementation")?.getattr("version")?;
+        let kind = info.getattr("releaselevel")?.extract::<String>()?;
+        let implementation_version: String = format!(
+            "{}.{}.{}{}",
+            info.getattr("major")?.extract::<usize>()?,
+            info.getattr("minor")?.extract::<usize>()?,
+            info.getattr("micro")?.extract::<usize>()?,
+            if kind == "final" {
+                String::new()
+            } else {
+                format!("{}{}", kind, info.getattr("serial")?.extract::<usize>()?)
+            }
+        );
+        let python_full_version: String = platform.getattr("python_version")?.call0()?.extract()?;
+        let python_version = format!("{}.{}", python_version_tuple.0, python_version_tuple.1);
+
+        // This is not written down in PEP 508, but it's the only reasonable assumption to make
+        let implementation_version =
+            StringVersion::from_str(&implementation_version).map_err(|err| {
+                PyValueError::new_err(format!(
+                    "Broken python implementation, implementation_version is not a valid PEP440 version: {err}"
+                ))
+            })?;
+        let python_full_version = StringVersion::from_str(&python_full_version).map_err(|err| {
+            PyValueError::new_err(format!(
+                "Broken python implementation, python_full_version is not a valid PEP440 version: {err}"
+            ))
+        })?;
+        let python_version = StringVersion::from_str(&python_version).map_err(|err| {
+            PyValueError::new_err(format!(
+                "Broken python implementation, python_version is not a valid PEP440 version: {err}"
+            ))
+        })?;
+        Ok(Self {
+            inner: Arc::new(MarkerEnvironmentInner {
+                implementation_name: name,
+                implementation_version,
+                os_name: os.getattr("name")?.extract()?,
+                platform_machine: platform.getattr("machine")?.call0()?.extract()?,
+                platform_python_implementation: platform
+                    .getattr("python_implementation")?
+                    .call0()?
+                    .extract()?,
+                platform_release: platform.getattr("release")?.call0()?.extract()?,
+                platform_system: platform.getattr("system")?.call0()?.extract()?,
+                platform_version: platform.getattr("version")?.call0()?.extract()?,
+                python_full_version,
+                python_version,
+                sys_platform: sys.getattr("platform")?.extract()?,
+            }),
+        })
+    }
+
+    /// Returns the name of the Python implementation for this environment.
+    #[getter]
+    pub fn py_implementation_name(&self) -> String {
+        self.implementation_name().to_string()
+    }
+
+    /// Returns the Python implementation version for this environment.
+    #[getter]
+    pub fn py_implementation_version(&self) -> StringVersion {
+        self.implementation_version().clone()
+    }
+
+    /// Returns the name of the operating system for this environment.
+    #[getter]
+    pub fn py_os_name(&self) -> String {
+        self.os_name().to_string()
+    }
+
+    /// Returns the name of the machine for this environment's platform.
+    #[getter]
+    pub fn py_platform_machine(&self) -> String {
+        self.platform_machine().to_string()
+    }
+
+    /// Returns the name of the Python implementation for this environment's
+    /// platform.
+    #[getter]
+    pub fn py_platform_python_implementation(&self) -> String {
+        self.platform_python_implementation().to_string()
+    }
+
+    /// Returns the release for this environment's platform.
+    #[getter]
+    pub fn py_platform_release(&self) -> String {
+        self.platform_release().to_string()
+    }
+
+    /// Returns the system for this environment's platform.
+    #[getter]
+    pub fn py_platform_system(&self) -> String {
+        self.platform_system().to_string()
+    }
+
+    /// Returns the version for this environment's platform.
+    #[getter]
+    pub fn py_platform_version(&self) -> String {
+        self.platform_version().to_string()
+    }
+
+    /// Returns the full version of Python for this environment.
+    #[getter]
+    pub fn py_python_full_version(&self) -> StringVersion {
+        self.python_full_version().clone()
+    }
+
+    /// Returns the version of Python for this environment.
+    #[getter]
+    pub fn py_python_version(&self) -> StringVersion {
+        self.python_version().clone()
+    }
+
+    /// Returns the name of the system platform for this environment.
+    #[getter]
+    pub fn py_sys_platform(&self) -> String {
+        self.sys_platform().to_string()
+    }
+}
+
+/// A builder for constructing a marker environment.
+///
+/// A value of this type can be fallibly converted to a full
+/// [`MarkerEnvironment`] via [`MarkerEnvironment::try_from`]. This can fail when
+/// the version strings given aren't valid.
+///
+/// The main utility of this type is for constructing dummy or test environment
+/// values.
+#[allow(missing_docs)]
+#[derive(Clone, Debug, Eq, Hash, PartialEq)]
+pub struct MarkerEnvironmentBuilder<'a> {
+    pub implementation_name: &'a str,
+    pub implementation_version: &'a str,
+    pub os_name: &'a str,
+    pub platform_machine: &'a str,
+    pub platform_python_implementation: &'a str,
+    pub platform_release: &'a str,
+    pub platform_system: &'a str,
+    pub platform_version: &'a str,
+    pub python_full_version: &'a str,
+    pub python_version: &'a str,
+    pub sys_platform: &'a str,
+}
+
+impl<'a> TryFrom<MarkerEnvironmentBuilder<'a>> for MarkerEnvironment {
+    type Error = VersionParseError;
+
+    fn try_from(builder: MarkerEnvironmentBuilder<'a>) -> Result<Self, Self::Error> {
+        Ok(MarkerEnvironment {
+            inner: Arc::new(MarkerEnvironmentInner {
+                implementation_name: builder.implementation_name.to_string(),
+                implementation_version: builder.implementation_version.parse()?,
+                os_name: builder.os_name.to_string(),
+                platform_machine: builder.platform_machine.to_string(),
+                platform_python_implementation: builder.platform_python_implementation.to_string(),
+                platform_release: builder.platform_release.to_string(),
+                platform_system: builder.platform_system.to_string(),
+                platform_version: builder.platform_version.to_string(),
+                python_full_version: builder.python_full_version.parse()?,
+                python_version: builder.python_version.parse()?,
+                sys_platform: builder.sys_platform.to_string(),
+            }),
+        })
+    }
+}

crates/pep508-rs/src/marker/mod.rs

@@ -0,0 +1,20 @@
+//! PEP 508 markers implementations with validation and warnings
+//!
+//! Markers allow you to install dependencies only in specific environments (python version,
+//! operating system, architecture, etc.) or when a specific feature is activated. E.g. you can
+//! say `importlib-metadata ; python_version < "3.8"` or
+//! `itsdangerous (>=1.1.0) ; extra == 'security'`. Unfortunately, the marker grammar has some
+//! oversights (e.g. <https://github.com/pypa/packaging.python.org/pull/1181>) and
+//! the design of comparisons (PEP 440 comparisons with lexicographic fallback) leads to confusing
+//! outcomes. This implementation tries to carefully validate everything and emit warnings whenever
+//! bogus comparisons with unintended semantics are made.
+
+mod environment;
+pub(crate) mod parse;
+mod tree;
+
+pub use environment::{MarkerEnvironment, MarkerEnvironmentBuilder};
+pub use tree::{
+    ExtraOperator, MarkerExpression, MarkerOperator, MarkerTree, MarkerValue, MarkerValueString,
+    MarkerValueVersion, MarkerWarningKind, StringVersion,
+};

crates/pep508-rs/src/marker/parse.rs

@@ -0,0 +1,527 @@
+use std::str::FromStr;
+
+use pep440_rs::{Version, VersionPattern, VersionSpecifier};
+use uv_normalize::ExtraName;
+
+use crate::cursor::Cursor;
+use crate::{
+    ExtraOperator, MarkerExpression, MarkerOperator, MarkerTree, MarkerValue, MarkerValueVersion,
+    MarkerWarningKind, Pep508Error, Pep508ErrorSource, Pep508Url, Reporter,
+};
+
+/// ```text
+/// version_cmp   = wsp* <'<=' | '<' | '!=' | '==' | '>=' | '>' | '~=' | '==='>
+/// marker_op     = version_cmp | (wsp* 'in') | (wsp* 'not' wsp+ 'in')
+/// ```
+/// The `wsp*` has already been consumed by the caller.
+fn parse_marker_operator<T: Pep508Url>(
+    cursor: &mut Cursor,
+) -> Result<MarkerOperator, Pep508Error<T>> {
+    let (start, len) = if cursor.peek_char().is_some_and(char::is_alphabetic) {
+        // "in" or "not"
+        cursor.take_while(|char| !char.is_whitespace() && char != '\'' && char != '"')
+    } else {
+        // A mathematical operator
+        cursor.take_while(|char| matches!(char, '<' | '=' | '>' | '~' | '!'))
+    };
+    let operator = cursor.slice(start, len);
+    if operator == "not" {
+        // 'not' wsp+ 'in'
+        match cursor.next() {
+            None => {
+                return Err(Pep508Error {
+                    message: Pep508ErrorSource::String(
+                        "Expected whitespace after 'not', found end of input".to_string(),
+                    ),
+                    start: cursor.pos(),
+                    len: 1,
+                    input: cursor.to_string(),
+                });
+            }
+            Some((_, whitespace)) if whitespace.is_whitespace() => {}
+            Some((pos, other)) => {
+                return Err(Pep508Error {
+                    message: Pep508ErrorSource::String(format!(
+                        "Expected whitespace after 'not', found '{other}'"
+                    )),
+                    start: pos,
+                    len: other.len_utf8(),
+                    input: cursor.to_string(),
+                });
+            }
+        };
+        cursor.eat_whitespace();
+        cursor.next_expect_char('i', cursor.pos())?;
+        cursor.next_expect_char('n', cursor.pos())?;
+        return Ok(MarkerOperator::NotIn);
+    }
+    MarkerOperator::from_str(operator).map_err(|_| Pep508Error {
+        message: Pep508ErrorSource::String(format!(
+            "Expected a valid marker operator (such as '>=' or 'not in'), found '{operator}'"
+        )),
+        start,
+        len,
+        input: cursor.to_string(),
+    })
+}
+
+/// Either a single or double quoted string or one of '`python_version`', '`python_full_version`',
+/// '`os_name`', '`sys_platform`', '`platform_release`', '`platform_system`', '`platform_version`',
+/// '`platform_machine`', '`platform_python_implementation`', '`implementation_name`',
+/// '`implementation_version`', 'extra'
+pub(crate) fn parse_marker_value<T: Pep508Url>(
+    cursor: &mut Cursor,
+) -> Result<MarkerValue, Pep508Error<T>> {
+    // > User supplied constants are always encoded as strings with either ' or " quote marks. Note
+    // > that backslash escapes are not defined, but existing implementations do support them. They
+    // > are not included in this specification because they add complexity and there is no observable
+    // > need for them today. Similarly we do not define non-ASCII character support: all the runtime
+    // > variables we are referencing are expected to be ASCII-only.
+    match cursor.peek() {
+        None => Err(Pep508Error {
+            message: Pep508ErrorSource::String(
+                "Expected marker value, found end of dependency specification".to_string(),
+            ),
+            start: cursor.pos(),
+            len: 1,
+            input: cursor.to_string(),
+        }),
+        // It can be a string ...
+        Some((start_pos, quotation_mark @ ('"' | '\''))) => {
+            cursor.next();
+            let (start, len) = cursor.take_while(|c| c != quotation_mark);
+            let value = cursor.slice(start, len).to_string();
+            cursor.next_expect_char(quotation_mark, start_pos)?;
+            Ok(MarkerValue::QuotedString(value))
+        }
+        // ... or it can be a keyword
+        Some(_) => {
+            let (start, len) = cursor.take_while(|char| {
+                !char.is_whitespace() && !['>', '=', '<', '!', '~', ')'].contains(&char)
+            });
+            let key = cursor.slice(start, len);
+            MarkerValue::from_str(key).map_err(|_| Pep508Error {
+                message: Pep508ErrorSource::String(format!(
+                    "Expected a valid marker name, found '{key}'"
+                )),
+                start,
+                len,
+                input: cursor.to_string(),
+            })
+        }
+    }
+}
+
+/// ```text
+/// marker_var:l marker_op:o marker_var:r
+/// ```
+pub(crate) fn parse_marker_key_op_value<T: Pep508Url>(
+    cursor: &mut Cursor,
+    reporter: &mut impl Reporter,
+) -> Result<MarkerExpression, Pep508Error<T>> {
+    cursor.eat_whitespace();
+    let l_value = parse_marker_value(cursor)?;
+    cursor.eat_whitespace();
+    // "not in" and "in" must be preceded by whitespace. We must already have matched a whitespace
+    // when we're here because other `parse_marker_key` would have pulled the characters in and
+    // errored
+    let operator = parse_marker_operator(cursor)?;
+    cursor.eat_whitespace();
+    let r_value = parse_marker_value(cursor)?;
+
+    // Convert a `<marker_value> <marker_op> <marker_value>` expression into it's
+    // typed equivalent.
+    let expr = match l_value {
+        // The only sound choice for this is `<version key> <version op> <quoted PEP 440 version>`
+        MarkerValue::MarkerEnvVersion(key) => {
+            let MarkerValue::QuotedString(value) = r_value else {
+                reporter.report(
+                    MarkerWarningKind::Pep440Error,
+                    format!(
+                        "Expected double quoted PEP 440 version to compare with {key},
+                        found {r_value}, will evaluate to false"
+                    ),
+                );
+
+                return Ok(MarkerExpression::arbitrary(
+                    MarkerValue::MarkerEnvVersion(key),
+                    operator,
+                    r_value,
+                ));
+            };
+
+            match parse_version_expr(key.clone(), operator, &value, reporter) {
+                Some(expr) => expr,
+                None => MarkerExpression::arbitrary(
+                    MarkerValue::MarkerEnvVersion(key),
+                    operator,
+                    MarkerValue::QuotedString(value),
+                ),
+            }
+        }
+        // The only sound choice for this is `<env key> <op> <string>`
+        MarkerValue::MarkerEnvString(key) => {
+            let value = match r_value {
+                MarkerValue::Extra
+                | MarkerValue::MarkerEnvVersion(_)
+                | MarkerValue::MarkerEnvString(_) => {
+                    reporter.report(
+                        MarkerWarningKind::MarkerMarkerComparison,
+                        "Comparing two markers with each other doesn't make any sense,
+                            will evaluate to false"
+                            .to_string(),
+                    );
+
+                    return Ok(MarkerExpression::arbitrary(
+                        MarkerValue::MarkerEnvString(key),
+                        operator,
+                        r_value,
+                    ));
+                }
+                MarkerValue::QuotedString(r_string) => r_string,
+            };
+
+            MarkerExpression::String {
+                key,
+                operator,
+                value,
+            }
+        }
+        // `extra == '...'`
+        MarkerValue::Extra => {
+            let value = match r_value {
+                MarkerValue::MarkerEnvVersion(_)
+                | MarkerValue::MarkerEnvString(_)
+                | MarkerValue::Extra => {
+                    reporter.report(
+                        MarkerWarningKind::ExtraInvalidComparison,
+                        "Comparing extra with something other than a quoted string is wrong,
+                            will evaluate to false"
+                            .to_string(),
+                    );
+
+                    return Ok(MarkerExpression::arbitrary(l_value, operator, r_value));
+                }
+                MarkerValue::QuotedString(value) => value,
+            };
+
+            match parse_extra_expr(operator, &value, reporter) {
+                Some(expr) => expr,
+                None => MarkerExpression::arbitrary(
+                    MarkerValue::Extra,
+                    operator,
+                    MarkerValue::QuotedString(value),
+                ),
+            }
+        }
+        // This is either MarkerEnvVersion, MarkerEnvString or Extra inverted
+        MarkerValue::QuotedString(l_string) => {
+            match r_value {
+                // The only sound choice for this is `<quoted PEP 440 version> <version op>` <version key>
+                MarkerValue::MarkerEnvVersion(key) => {
+                    match parse_inverted_version_expr(&l_string, operator, key.clone(), reporter) {
+                        Some(expr) => expr,
+                        None => MarkerExpression::arbitrary(
+                            MarkerValue::QuotedString(l_string),
+                            operator,
+                            MarkerValue::MarkerEnvVersion(key),
+                        ),
+                    }
+                }
+                // '...' == <env key>
+                MarkerValue::MarkerEnvString(key) => MarkerExpression::String {
+                    key,
+                    // Invert the operator to normalize the expression order.
+                    operator: operator.invert(),
+                    value: l_string,
+                },
+                // `'...' == extra`
+                MarkerValue::Extra => match parse_extra_expr(operator, &l_string, reporter) {
+                    Some(expr) => expr,
+                    None => MarkerExpression::arbitrary(
+                        MarkerValue::QuotedString(l_string),
+                        operator,
+                        MarkerValue::Extra,
+                    ),
+                },
+                // `'...' == '...'`, doesn't make much sense
+                MarkerValue::QuotedString(_) => {
+                    // Not even pypa/packaging 22.0 supports this
+                    // https://github.com/pypa/packaging/issues/632
+                    let expr = MarkerExpression::arbitrary(
+                        MarkerValue::QuotedString(l_string),
+                        operator,
+                        r_value,
+                    );
+
+                    reporter.report(
+                        MarkerWarningKind::StringStringComparison,
+                        format!(
+                            "Comparing two quoted strings with each other doesn't make sense:
+                            {expr}, will evaluate to false"
+                        ),
+                    );
+
+                    expr
+                }
+            }
+        }
+    };
+
+    Ok(expr)
+}
+
+/// Creates an instance of [`MarkerExpression::Version`] with the given values.
+///
+/// Reports a warning on failure, and returns `None`.
+fn parse_version_expr(
+    key: MarkerValueVersion,
+    marker_operator: MarkerOperator,
+    value: &str,
+    reporter: &mut impl Reporter,
+) -> Option<MarkerExpression> {
+    let pattern = match value.parse::<VersionPattern>() {
+        Ok(pattern) => pattern,
+        Err(err) => {
+            reporter.report(
+                MarkerWarningKind::Pep440Error,
+                format!(
+                    "Expected PEP 440 version to compare with {key}, found {value},
+                    will evaluate to false: {err}"
+                ),
+            );
+
+            return None;
+        }
+    };
+
+    let Some(operator) = marker_operator.to_pep440_operator() else {
+        reporter.report(
+            MarkerWarningKind::Pep440Error,
+            format!(
+                "Expected PEP 440 version operator to compare {key} with '{version}',
+                    found '{marker_operator}', will evaluate to false",
+                version = pattern.version()
+            ),
+        );
+
+        return None;
+    };
+
+    let specifier = match VersionSpecifier::from_pattern(operator, pattern) {
+        Ok(specifier) => specifier,
+        Err(err) => {
+            reporter.report(
+                MarkerWarningKind::Pep440Error,
+                format!("Invalid operator/version combination: {err}"),
+            );
+            return None;
+        }
+    };
+
+    Some(MarkerExpression::Version { key, specifier })
+}
+
+/// Creates an instance of [`MarkerExpression::Version`] from an inverted expression.
+///
+/// Reports a warning on failure, and returns `None`.
+fn parse_inverted_version_expr(
+    value: &str,
+    marker_operator: MarkerOperator,
+    key: MarkerValueVersion,
+    reporter: &mut impl Reporter,
+) -> Option<MarkerExpression> {
+    // Invert the operator to normalize the expression order.
+    let marker_operator = marker_operator.invert();
+
+    // Not star allowed here, `'3.*' == python_version` is not a valid PEP 440 comparison.
+    let version = match value.parse::<Version>() {
+        Ok(version) => version,
+        Err(err) => {
+            reporter.report(
+                MarkerWarningKind::Pep440Error,
+                format!(
+                    "Expected PEP 440 version to compare with {key}, found {value},
+                    will evaluate to false: {err}"
+                ),
+            );
+
+            return None;
+        }
+    };
+
+    let Some(operator) = marker_operator.to_pep440_operator() else {
+        reporter.report(
+            MarkerWarningKind::Pep440Error,
+            format!(
+                "Expected PEP 440 version operator to compare {key} with '{version}',
+                    found '{marker_operator}', will evaluate to false"
+            ),
+        );
+
+        return None;
+    };
+
+    let specifier = match VersionSpecifier::from_version(operator, version) {
+        Ok(specifier) => specifier,
+        Err(err) => {
+            reporter.report(
+                MarkerWarningKind::Pep440Error,
+                format!("Invalid operator/version combination: {err}"),
+            );
+            return None;
+        }
+    };
+
+    Some(MarkerExpression::Version { key, specifier })
+}
+
+/// Creates an instance of [`MarkerExpression::Extra`] with the given values, falling back to
+/// [`MarkerExpression::Arbitrary`] on failure.
+fn parse_extra_expr(
+    operator: MarkerOperator,
+    value: &str,
+    reporter: &mut impl Reporter,
+) -> Option<MarkerExpression> {
+    let name = match ExtraName::from_str(value) {
+        Ok(name) => name,
+        Err(err) => {
+            reporter.report(
+                MarkerWarningKind::ExtraInvalidComparison,
+                format!("Expected extra name, found '{value}', will evaluate to false: {err}"),
+            );
+
+            return None;
+        }
+    };
+
+    if let Some(operator) = ExtraOperator::from_marker_operator(operator) {
+        return Some(MarkerExpression::Extra { operator, name });
+    }
+
+    reporter.report(
+        MarkerWarningKind::ExtraInvalidComparison,
+        "Comparing extra with something other than a quoted string is wrong,
+        will evaluate to false"
+            .to_string(),
+    );
+    None
+}
+
+/// ```text
+/// marker_expr   = marker_var:l marker_op:o marker_var:r -> (o, l, r)
+///               | wsp* '(' marker:m wsp* ')' -> m
+/// ```
+fn parse_marker_expr<T: Pep508Url>(
+    cursor: &mut Cursor,
+    reporter: &mut impl Reporter,
+) -> Result<MarkerTree, Pep508Error<T>> {
+    cursor.eat_whitespace();
+    if let Some(start_pos) = cursor.eat_char('(') {
+        let marker = parse_marker_or(cursor, reporter)?;
+        cursor.next_expect_char(')', start_pos)?;
+        Ok(marker)
+    } else {
+        Ok(MarkerTree::Expression(parse_marker_key_op_value(
+            cursor, reporter,
+        )?))
+    }
+}
+
+/// ```text
+/// marker_and    = marker_expr:l wsp* 'and' marker_expr:r -> ('and', l, r)
+///               | marker_expr:m -> m
+/// ```
+fn parse_marker_and<T: Pep508Url>(
+    cursor: &mut Cursor,
+    reporter: &mut impl Reporter,
+) -> Result<MarkerTree, Pep508Error<T>> {
+    parse_marker_op(cursor, "and", MarkerTree::And, parse_marker_expr, reporter)
+}
+
+/// ```text
+/// marker_or     = marker_and:l wsp* 'or' marker_and:r -> ('or', l, r)
+///                   | marker_and:m -> m
+/// ```
+fn parse_marker_or<T: Pep508Url>(
+    cursor: &mut Cursor,
+    reporter: &mut impl Reporter,
+) -> Result<MarkerTree, Pep508Error<T>> {
+    parse_marker_op(cursor, "or", MarkerTree::Or, parse_marker_and, reporter)
+}
+
+/// Parses both `marker_and` and `marker_or`
+fn parse_marker_op<T: Pep508Url, R: Reporter>(
+    cursor: &mut Cursor,
+    op: &str,
+    op_constructor: fn(Vec<MarkerTree>) -> MarkerTree,
+    parse_inner: fn(&mut Cursor, &mut R) -> Result<MarkerTree, Pep508Error<T>>,
+    reporter: &mut R,
+) -> Result<MarkerTree, Pep508Error<T>> {
+    // marker_and or marker_expr
+    let first_element = parse_inner(cursor, reporter)?;
+    // wsp*
+    cursor.eat_whitespace();
+    // Check if we're done here instead of invoking the whole vec allocating loop
+    if matches!(cursor.peek_char(), None | Some(')')) {
+        return Ok(first_element);
+    }
+
+    let mut expressions = Vec::with_capacity(1);
+    expressions.push(first_element);
+    loop {
+        // wsp*
+        cursor.eat_whitespace();
+        // ('or' marker_and) or ('and' marker_or)
+        let (start, len) = cursor.peek_while(|c| !c.is_whitespace());
+        match cursor.slice(start, len) {
+            value if value == op => {
+                cursor.take_while(|c| !c.is_whitespace());
+                let expression = parse_inner(cursor, reporter)?;
+                expressions.push(expression);
+            }
+            _ => {
+                // Build minimal trees
+                return if expressions.len() == 1 {
+                    Ok(expressions.remove(0))
+                } else {
+                    Ok(op_constructor(expressions))
+                };
+            }
+        }
+    }
+}
+
+/// ```text
+/// marker        = marker_or^
+/// ```
+pub(crate) fn parse_markers_cursor<T: Pep508Url>(
+    cursor: &mut Cursor,
+    reporter: &mut impl Reporter,
+) -> Result<MarkerTree, Pep508Error<T>> {
+    let marker = parse_marker_or(cursor, reporter)?;
+    cursor.eat_whitespace();
+    if let Some((pos, unexpected)) = cursor.next() {
+        // If we're here, both parse_marker_or and parse_marker_and returned because the next
+        // character was neither "and" nor "or"
+        return Err(Pep508Error {
+            message: Pep508ErrorSource::String(format!(
+                "Unexpected character '{unexpected}', expected 'and', 'or' or end of input"
+            )),
+            start: pos,
+            len: cursor.remaining(),
+            input: cursor.to_string(),
+        });
+    };
+    Ok(marker)
+}
+
+/// Parses markers such as `python_version < '3.8'` or
+/// `python_version == "3.10" and (sys_platform == "win32" or (os_name == "linux" and implementation_name == 'cpython'))`
+pub(crate) fn parse_markers<T: Pep508Url>(
+    markers: &str,
+    reporter: &mut impl Reporter,
+) -> Result<MarkerTree, Pep508Error<T>> {
+    let mut chars = Cursor::new(markers);
+    parse_markers_cursor(&mut chars, reporter)
+}

crates/pep508-rs/src/unnamed.rs

@@ -6,7 +6,7 @@ use std::str::FromStr;
 use uv_fs::normalize_url_path;
 use uv_normalize::ExtraName;
 
-use crate::marker::parse_markers_cursor;
+use crate::marker::parse;
 use crate::{
     expand_env_vars, parse_extras_cursor, split_extras, split_scheme, strip_host, Cursor,
     MarkerEnvironment, MarkerTree, Pep508Error, Pep508ErrorSource, Pep508Url, Reporter,
@@ -172,7 +172,7 @@ fn parse_unnamed_requirement<Url: UnnamedRequirementUrl>(
     let marker = if cursor.peek_char() == Some(';') {
         // Skip past the semicolon
         cursor.next();
-        Some(parse_markers_cursor(cursor, reporter)?)
+        Some(parse::parse_markers_cursor(cursor, reporter)?)
     } else {
         None
     };