ronantakizawa/codereview-bench huggingface dataset:
Repository: astral-sh/uv
File: crates/uv/src/commands/pip/install.rs (Rust)

Code before:
            &flat_index,
            state.index(),
            &build_dispatch,
            concurrency,
            options,
            Box::new(DefaultResolveLogger),
            printer,
        )
        .await
        {
            Ok(graph) => Resolution::from(graph),
            Err(err) => {
                return diagnostics::OperationDiagnostic::native_tls(network_settings.native_tls)
                    .report(err)
                    .map_or(Ok(ExitStatus::Failure), |err| Err(err.into()));
            }
        };

        (resolution, hasher)
    };

    // Constrain any build requirements marked as `match-runtime = true`.
    let extra_build_requires = extra_build_requires.match_runtime(&resolution)?;

    // Create a build dispatch.
    let build_dispatch = BuildDispatch::new(
        &client,
        &cache,
        build_constraints.clone(),
        interpreter,
        &index_locations,
        &flat_index,
        &dependency_metadata,
        state.clone(),
        index_strategy,
        config_settings,
        config_settings_package,
        build_isolation,
        &extra_build_requires,
        link_mode,
        &build_options,
        &hasher,
        exclude_newer.clone(),
        sources,
        WorkspaceCache::default(),
        concurrency,
        preview,
    );

    // Sync the environment.
    match operations::install(

Review comment (suggestion):
This is `build_dispatch.with_extra_build_requires(&extra_build_requires)` but the lifetimes get rejected. I'll try again.

Code after:
            &flat_index,
            state.index(),
            &build_dispatch,
            concurrency,
            options,
            Box::new(DefaultResolveLogger),
            printer,
        )
        .await
        {
            Ok(graph) => Resolution::from(graph),
            Err(err) => {
                return diagnostics::OperationDiagnostic::native_tls(network_settings.native_tls)
                    .report(err)
                    .map_or(Ok(ExitStatus::Failure), |err| Err(err.into()));
            }
        };

        (resolution, hasher)
    };

    // Constrain any build requirements marked as `match-runtime = true`.
    let extra_build_requires = extra_build_requires.match_runtime(&resolution)?;

    // Create a build dispatch.
    let build_dispatch = BuildDispatch::new(
        &client,
        &cache,
        &build_constraints,
        interpreter,
        &index_locations,
        &flat_index,
        &dependency_metadata,
        state.clone(),
        index_strategy,
        config_settings,
        config_settings_package,
        build_isolation,
        &extra_build_requires,
        link_mode,
        &build_options,
        &hasher,
        exclude_newer.clone(),
        sources,
        WorkspaceCache::default(),
        concurrency,
        preview,
    );

    // Sync the environment.
    match operations::install(
Repository: astral-sh/uv
File: crates/uv-platform/src/os.rs (Rust)

Code before:
        }
        if self.is_windows() && other.is_emscripten() {
            return false;
        }

        // Otherwise, we require an exact match
        *self == other
    }
}

impl Display for Os {
    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
        match &**self {
            target_lexicon::OperatingSystem::Darwin(_) => write!(f, "macos"),
            inner => write!(f, "{inner}"),
        }
    }
}

impl FromStr for Os {
    type Err = Error;

    fn from_str(s: &str) -> Result<Self, Self::Err> {
        let inner = match s {
            "macos" => target_lexicon::OperatingSystem::Darwin(None),
            "pyodide" => target_lexicon::OperatingSystem::Emscripten,
            _ => target_lexicon::OperatingSystem::from_str(s)
                .map_err(|()| Error::UnknownOs(s.to_string()))?,
        };
        if matches!(inner, target_lexicon::OperatingSystem::Unknown) {
            return Err(Error::UnknownOs(s.to_string()));
        }
        Ok(Self(inner))
    }
}

impl Deref for Os {
    type Target = target_lexicon::OperatingSystem;

    fn deref(&self) -> &Self::Target {
        &self.0
    }
}

impl From<&uv_platform_tags::Os> for Os {
    fn from(value: &uv_platform_tags::Os) -> Self {
        match value {
            uv_platform_tags::Os::Dragonfly { .. } => {
                Self::new(target_lexicon::OperatingSystem::Dragonfly)
            }
            uv_platform_tags::Os::FreeBsd { .. } => {

Review comment (suggestion):
I think this is superfluous now. I started down this route but then decided I should make it an `ImplementationName`

Code after:
        }
        if self.is_windows() && other.is_emscripten() {
            return false;
        }

        // Otherwise, we require an exact match
        *self == other
    }
}

impl Display for Os {
    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
        match &**self {
            target_lexicon::OperatingSystem::Darwin(_) => write!(f, "macos"),
            inner => write!(f, "{inner}"),
        }
    }
}

impl FromStr for Os {
    type Err = Error;

    fn from_str(s: &str) -> Result<Self, Self::Err> {
        let inner = match s {
            "macos" => target_lexicon::OperatingSystem::Darwin(None),
            _ => target_lexicon::OperatingSystem::from_str(s)
                .map_err(|()| Error::UnknownOs(s.to_string()))?,
        };
        if matches!(inner, target_lexicon::OperatingSystem::Unknown) {
            return Err(Error::UnknownOs(s.to_string()));
        }
        Ok(Self(inner))
    }
}

impl Deref for Os {
    type Target = target_lexicon::OperatingSystem;

    fn deref(&self) -> &Self::Target {
        &self.0
    }
}

impl From<&uv_platform_tags::Os> for Os {
    fn from(value: &uv_platform_tags::Os) -> Self {
        match value {
            uv_platform_tags::Os::Dragonfly { .. } => {
                Self::new(target_lexicon::OperatingSystem::Dragonfly)
            }
            uv_platform_tags::Os::FreeBsd { .. } => {
                Self::new(target_lexicon::OperatingSystem::Freebsd)
Repository: astral-sh/uv
File: crates/uv/tests/it/format.rs (Rust)

Code before:
    let context = TestContext::new_with_versions(&[]);

    let pyproject_toml = context.temp_dir.child("pyproject.toml");
    pyproject_toml.write_str(indoc! {r#"
        [project]
        name = "project"
        version = "0.1.0"
        requires-python = ">=3.12"
        dependencies = []
    "#})?;

    // Create an unformatted Python file
    let main_py = context.temp_dir.child("main.py");
    main_py.write_str(indoc! {r#"
        import sys
        def   hello():
            print(  "Hello, World!"  )
        if __name__=="__main__":
            hello(   )
    "#})?;

    let subdir = context.temp_dir.child("subdir");
    fs_err::create_dir_all(&subdir)?;

    uv_snapshot!(context.filters(), context.format().current_dir(&subdir), @r"
    success: true
    exit_code: 0
    ----- stdout -----
    1 file reformatted

    ----- stderr -----
    warning: `uv format` is experimental and may change without warning. Pass `--preview-features format` to disable this warning.
    ");

    // Check that the file was formatted
    let formatted_content = fs_err::read_to_string(&main_py)?;
    assert_snapshot!(formatted_content, @r#"
    import sys


    def hello():
        print("Hello, World!")


    if __name__ == "__main__":
        hello()
    "#);

    Ok(())
}
#[test]

Review comment (suggestion):

```suggestion
    // Using format from a subdirectory should still run in the project root
    uv_snapshot!(context.filters(), context.format().current_dir(&subdir), @r"
```

Code after:
    let context = TestContext::new_with_versions(&[]);

    let pyproject_toml = context.temp_dir.child("pyproject.toml");
    pyproject_toml.write_str(indoc! {r#"
        [project]
        name = "project"
        version = "0.1.0"
        requires-python = ">=3.12"
        dependencies = []
    "#})?;

    // Create an unformatted Python file
    let main_py = context.temp_dir.child("main.py");
    main_py.write_str(indoc! {r#"
        import sys
        def   hello():
            print(  "Hello, World!"  )
        if __name__=="__main__":
            hello(   )
    "#})?;

    let subdir = context.temp_dir.child("subdir");
    fs_err::create_dir_all(&subdir)?;

    // Using format from a subdirectory should still run in the project root
    uv_snapshot!(context.filters(), context.format().current_dir(&subdir), @r"
    success: true
    exit_code: 0
    ----- stdout -----
    1 file reformatted

    ----- stderr -----
    warning: `uv format` is experimental and may change without warning. Pass `--preview-features format` to disable this warning.
    ");

    // Check that the file was formatted
    let formatted_content = fs_err::read_to_string(&main_py)?;
    assert_snapshot!(formatted_content, @r#"
    import sys


    def hello():
        print("Hello, World!")


    if __name__ == "__main__":
        hello()
    "#);

    Ok(())
}
Repository: astral-sh/uv
File: crates/uv/tests/it/format.rs (Rust)

Code before:
    success: true
    exit_code: 0
    ----- stdout -----
    1 file reformatted

    ----- stderr -----
    warning: `uv format` is experimental and may change without warning. Pass `--preview-features format` to disable this warning.
    ");

    // Check that the file was formatted
    let formatted_content = fs_err::read_to_string(&main_py)?;
    assert_snapshot!(formatted_content, @r#"
    import sys


    def hello():
        print("Hello, World!")


    if __name__ == "__main__":
        hello()
    "#);

    Ok(())
}
#[test]
fn format_relative_project() -> Result<()> {
    let context = TestContext::new_with_versions(&[]);

    let pyproject_toml = context.temp_dir.child("project").child("pyproject.toml");
    pyproject_toml.write_str(indoc! {r#"
        [project]
        name = "project"
        version = "0.1.0"
        requires-python = ">=3.12"
        dependencies = []
    "#})?;

    // Create an unformatted Python file in the relative project
    let relative_project_main_py = context.temp_dir.child("project").child("main.py");
    relative_project_main_py.write_str(indoc! {r#"
        import sys
        def   hello():
            print(  "Hello, World!"  )
        if __name__=="__main__":
            hello(   )
    "#})?;

    // Create another unformatted Python file in the root directory
    let root_main_py = context.temp_dir.child("main.py");
    root_main_py.write_str(indoc! {r#"

Review comment (suggestion):
We should have a newline here
```suggestion
}

#[test]
```

Code after:
    uv_snapshot!(context.filters(), context.format().current_dir(&subdir), @r"
    success: true
    exit_code: 0
    ----- stdout -----
    1 file reformatted

    ----- stderr -----
    warning: `uv format` is experimental and may change without warning. Pass `--preview-features format` to disable this warning.
    ");

    // Check that the file was formatted
    let formatted_content = fs_err::read_to_string(&main_py)?;
    assert_snapshot!(formatted_content, @r#"
    import sys


    def hello():
        print("Hello, World!")


    if __name__ == "__main__":
        hello()
    "#);

    Ok(())
}

#[test]
fn format_relative_project() -> Result<()> {
    let context = TestContext::new_with_versions(&[]);

    let pyproject_toml = context.temp_dir.child("project").child("pyproject.toml");
    pyproject_toml.write_str(indoc! {r#"
        [project]
        name = "project"
        version = "0.1.0"
        requires-python = ">=3.12"
        dependencies = []
    "#})?;

    // Create an unformatted Python file in the relative project
    let relative_project_main_py = context.temp_dir.child("project").child("main.py");
    relative_project_main_py.write_str(indoc! {r#"
        import sys
        def   hello():
            print(  "Hello, World!"  )
        if __name__=="__main__":
            hello(   )
    "#})?;

    // Create another unformatted Python file in the root directory
Repository: astral-sh/uv
File: crates/uv-distribution-types/src/installed.rs (Rust)

Code before:
    InvalidEggLinkPath(PathBuf),

    #[error("Invalid .egg-link target: `{}`", _0.user_display())]
    InvalidEggLinkTarget(PathBuf),

    #[error("Failed to parse METADATA file: `{}`", path.user_display())]
    MetadataParse {
        path: PathBuf,
        #[source]
        err: Box<MetadataError>,
    },

    #[error("Failed to parse `PKG-INFO` file: `{}`", path.user_display())]
    PkgInfoParse {
        path: PathBuf,
        #[source]
        err: Box<MetadataError>,
    },
}

#[derive(Debug, Clone)]
pub struct InstalledDist {
    pub kind: InstalledDistKind,
    metadata_cache: OnceLock<uv_pypi_types::ResolutionMetadata>,
    tags_cache: OnceLock<Option<ExpandedTags>>,
}

impl From<InstalledDistKind> for InstalledDist {
    fn from(kind: InstalledDistKind) -> Self {
        Self {
            kind,
            metadata_cache: OnceLock::new(),
            tags_cache: OnceLock::new(),
        }
    }
}

impl std::hash::Hash for InstalledDist {
    fn hash<H: std::hash::Hasher>(&self, state: &mut H) {
        self.kind.hash(state);
    }
}

impl PartialEq for InstalledDist {
    fn eq(&self, other: &Self) -> bool {
        self.kind == other.kind
    }
}

impl Eq for InstalledDist {}


Review comment (performance):
Usually, I prefer centralized caches for non-pure data, it took me a bit to realize that this works as `InstalledDist` gets invalidated anyway if we modify the environment.

Code after:
    InvalidEggLinkPath(PathBuf),

    #[error("Invalid .egg-link target: `{}`", _0.user_display())]
    InvalidEggLinkTarget(PathBuf),

    #[error("Failed to parse METADATA file: `{}`", path.user_display())]
    MetadataParse {
        path: PathBuf,
        #[source]
        err: Box<MetadataError>,
    },

    #[error("Failed to parse `PKG-INFO` file: `{}`", path.user_display())]
    PkgInfoParse {
        path: PathBuf,
        #[source]
        err: Box<MetadataError>,
    },
}

#[derive(Debug, Clone)]
pub struct InstalledDist {
    pub kind: InstalledDistKind,
    // Cache data that must be read from the `.dist-info` directory. These are safe to cache as
    // the `InstalledDist` is immutable after creation.
    metadata_cache: OnceLock<uv_pypi_types::ResolutionMetadata>,
    tags_cache: OnceLock<Option<ExpandedTags>>,
}

impl From<InstalledDistKind> for InstalledDist {
    fn from(kind: InstalledDistKind) -> Self {
        Self {
            kind,
            metadata_cache: OnceLock::new(),
            tags_cache: OnceLock::new(),
        }
    }
}

impl std::hash::Hash for InstalledDist {
    fn hash<H: std::hash::Hasher>(&self, state: &mut H) {
        self.kind.hash(state);
    }
}

impl PartialEq for InstalledDist {
    fn eq(&self, other: &Self) -> bool {
        self.kind == other.kind
    }
}
Repository: astral-sh/uv
File: crates/uv-platform-tags/src/tags.rs (Rust)

Code before:
    Hash,
    rkyv::Archive,
    rkyv::Deserialize,
    rkyv::Serialize,
)]
#[rkyv(derive(Debug))]
pub enum AndroidArch {
    ArmeabiV7a,
    Arm64V8a,
    X86,
    X86_64,
}

impl std::fmt::Display for AndroidArch {
    fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
        write!(f, "{}", self.name())
    }
}

impl FromStr for AndroidArch {
    type Err = String;

    fn from_str(s: &str) -> Result<Self, Self::Err> {
        match s {
            "armeabi_v7a" => Ok(Self::ArmeabiV7a),
            "arm64_v8a" => Ok(Self::Arm64V8a),
            "x86" => Ok(Self::X86),
            "x86_64" => Ok(Self::X86_64),
            _ => Err(format!("Invalid Android arch format: {s}")),
        }
    }
}

impl AndroidArch {
    /// Determine the appropriate multiarch for a iOS version.
    pub fn from_arch(arch: Arch) -> Self {
        match arch {
            Arch::Aarch64 => Self::Arm64V8a,
            Arch::Armv7L => Self::ArmeabiV7a,
            Arch::X86 => Self::X86,
            Arch::X86_64 => Self::X86_64,
            _ => unreachable!(),
        }
    }

    /// Return the canonical name of the binary format.
    pub fn name(self) -> &'static str {
        match self {
            Self::ArmeabiV7a => "armeabi_v7a",
            Self::Arm64V8a => "arm64_v8a",
            Self::X86 => "x86",

Review comment (suggestion):
Is there a link we could include for the supported values here (e.g., to documentation)? I see `arm64_v8a`  and `x86_64` in the linked issue (https://github.com/astral-sh/uv/issues/8029), but not the other two.

Code after:
    Hash,
    rkyv::Archive,
    rkyv::Deserialize,
    rkyv::Serialize,
)]
#[rkyv(derive(Debug))]
pub enum AndroidAbi {
    // Source: https://packaging.python.org/en/latest/specifications/platform-compatibility-tags/#android
    ArmeabiV7a,
    Arm64V8a,
    X86,
    X86_64,
}

impl std::fmt::Display for AndroidAbi {
    fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
        write!(f, "{}", self.name())
    }
}

impl FromStr for AndroidAbi {
    type Err = String;

    fn from_str(s: &str) -> Result<Self, Self::Err> {
        match s {
            "armeabi_v7a" => Ok(Self::ArmeabiV7a),
            "arm64_v8a" => Ok(Self::Arm64V8a),
            "x86" => Ok(Self::X86),
            "x86_64" => Ok(Self::X86_64),
            _ => Err(format!("Invalid Android arch format: {s}")),
        }
    }
}

impl AndroidAbi {
    /// Determine the appropriate Android arch.
    pub fn from_arch(arch: Arch) -> Result<Self, String> {
        match arch {
            Arch::Aarch64 => Ok(Self::Arm64V8a),
            Arch::Armv7L => Ok(Self::ArmeabiV7a),
            Arch::X86 => Ok(Self::X86),
            Arch::X86_64 => Ok(Self::X86_64),
            _ => Err(format!("Invalid Android arch format: {arch}")),
        }
    }

    /// Return the canonical name of the binary format.
    pub fn name(self) -> &'static str {
        match self {
            Self::ArmeabiV7a => "armeabi_v7a",
            Self::Arm64V8a => "arm64_v8a",
Repository: astral-sh/uv
File: crates/uv-platform-tags/src/tags.rs (Rust)

Code before:
            Self::Intel => "intel",
            Self::Ppc => "ppc",
            Self::Ppc64 => "ppc64",
            Self::Universal => "universal",
            Self::Universal2 => "universal2",
            Self::X86_64 => "x86_64",
        }
    }
}

#[derive(
    Debug,
    Copy,
    Clone,
    Eq,
    PartialEq,
    Ord,
    PartialOrd,
    Hash,
    rkyv::Archive,
    rkyv::Deserialize,
    rkyv::Serialize,
)]
#[rkyv(derive(Debug))]
pub enum AndroidArch {
    // Source: https://peps.python.org/pep-0738/#architectures
    ArmeabiV7a,
    Arm64V8a,
    X86,
    X86_64,
}

impl std::fmt::Display for AndroidArch {
    fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
        write!(f, "{}", self.name())
    }
}

impl FromStr for AndroidArch {
    type Err = String;

    fn from_str(s: &str) -> Result<Self, Self::Err> {
        match s {
            "armeabi_v7a" => Ok(Self::ArmeabiV7a),
            "arm64_v8a" => Ok(Self::Arm64V8a),
            "x86" => Ok(Self::X86),
            "x86_64" => Ok(Self::X86_64),
            _ => Err(format!("Invalid Android arch format: {s}")),
        }
    }
}

Review comment (suggestion):
```suggestion
    // Source: https://packaging.python.org/en/latest/specifications/platform-compatibility-tags/#android
```

Code after:
            Self::Ppc => "ppc",
            Self::Ppc64 => "ppc64",
            Self::Universal => "universal",
            Self::Universal2 => "universal2",
            Self::X86_64 => "x86_64",
        }
    }
}

#[derive(
    Debug,
    Copy,
    Clone,
    Eq,
    PartialEq,
    Ord,
    PartialOrd,
    Hash,
    rkyv::Archive,
    rkyv::Deserialize,
    rkyv::Serialize,
)]
#[rkyv(derive(Debug))]
pub enum AndroidAbi {
    // Source: https://packaging.python.org/en/latest/specifications/platform-compatibility-tags/#android
    ArmeabiV7a,
    Arm64V8a,
    X86,
    X86_64,
}

impl std::fmt::Display for AndroidAbi {
    fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
        write!(f, "{}", self.name())
    }
}

impl FromStr for AndroidAbi {
    type Err = String;

    fn from_str(s: &str) -> Result<Self, Self::Err> {
        match s {
            "armeabi_v7a" => Ok(Self::ArmeabiV7a),
            "arm64_v8a" => Ok(Self::Arm64V8a),
            "x86" => Ok(Self::X86),
            "x86_64" => Ok(Self::X86_64),
            _ => Err(format!("Invalid Android arch format: {s}")),
        }
    }
}
Repository: astral-sh/uv
File: crates/uv-configuration/src/target_triple.rs (Rust)

Code before:
    pub fn markers(self, base: &MarkerEnvironment) -> MarkerEnvironment {
        base.clone()
            .with_os_name(self.os_name())
            .with_platform_machine(self.platform_machine())
            .with_platform_system(self.platform_system())
            .with_sys_platform(self.sys_platform())
            .with_platform_release(self.platform_release())
            .with_platform_version(self.platform_version())
    }
}

/// Return the macOS deployment target as parsed from the environment.
fn macos_deployment_target() -> Option<(u16, u16)> {
    let version = std::env::var(EnvVars::MACOSX_DEPLOYMENT_TARGET).ok()?;
    let mut parts = version.split('.');

    // Parse the major version (e.g., `12` in `12.0`).
    let major = parts.next()?.parse::<u16>().ok()?;

    // Parse the minor version (e.g., `0` in `12.0`), with a default of `0`.
    let minor = parts.next().unwrap_or("0").parse::<u16>().ok()?;

    Some((major, minor))
}

/// Return the macOS deployment target as parsed from the environment.
fn android_api_level() -> Option<u16> {
    let api_level_str = std::env::var(EnvVars::ANDROID_API_LEVEL).ok()?;

    // Parse the api level.
    let api_level = api_level_str.parse::<u16>().ok()?;

    Some(api_level)
}


Review comment (suggestion):
```suggestion
/// Return the Android API level as parsed from the environment.
```

Code after:
    pub fn markers(self, base: &MarkerEnvironment) -> MarkerEnvironment {
        base.clone()
            .with_os_name(self.os_name())
            .with_platform_machine(self.platform_machine())
            .with_platform_system(self.platform_system())
            .with_sys_platform(self.sys_platform())
            .with_platform_release(self.platform_release())
            .with_platform_version(self.platform_version())
    }
}

/// Return the macOS deployment target as parsed from the environment.
fn macos_deployment_target() -> Option<(u16, u16)> {
    let version = std::env::var(EnvVars::MACOSX_DEPLOYMENT_TARGET).ok()?;
    let mut parts = version.split('.');

    // Parse the major version (e.g., `12` in `12.0`).
    let major = parts.next()?.parse::<u16>().ok()?;

    // Parse the minor version (e.g., `0` in `12.0`), with a default of `0`.
    let minor = parts.next().unwrap_or("0").parse::<u16>().ok()?;

    Some((major, minor))
}

/// Return the Android API level as parsed from the environment.
fn android_api_level() -> Option<u16> {
    let api_level_str = std::env::var(EnvVars::ANDROID_API_LEVEL).ok()?;

    // Parse the api level.
    let api_level = api_level_str.parse::<u16>().ok()?;

    Some(api_level)
}