Skip to content

refactor: move more platform-specific code under sys#735

Merged
reubeno merged 7 commits into
mainfrom
shuffle-cfg-code
Nov 6, 2025
Merged

refactor: move more platform-specific code under sys#735
reubeno merged 7 commits into
mainfrom
shuffle-cfg-code

Conversation

@reubeno
Copy link
Copy Markdown
Owner

@reubeno reubeno commented Nov 6, 2025

No description provided.

Comment thread brush-core/src/sys/stubs/commands.rs Fixed
reubeno and others added 3 commits November 5, 2025 22:33
Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
@github-actions
Copy link
Copy Markdown

github-actions Bot commented Nov 6, 2025

Public API changes for crate: brush-core

Removed items

-pub brush_core::error::ErrorKind::ErrnoError(nix::errno::consts::Errno)
-pub brush_core::error::ErrorKind::ProcfsError(procfs_core::ProcError)
-pub brush_core::ErrorKind::ErrnoError(nix::errno::consts::Errno)
-pub brush_core::ErrorKind::ProcfsError(procfs_core::ProcError)

Added items

+pub brush_core::error::ErrorKind::NotSupportedOnThisPlatform(&'static str)
+pub brush_core::error::ErrorKind::PlatformError(brush_core::sys::PlatformError)
+impl core::convert::From<nix::errno::consts::Errno> for brush_core::error::ErrorKind
+impl core::convert::From<nix::errno::consts::Errno> for brush_core::error::ErrorKind
+pub fn brush_core::error::ErrorKind::from(err: nix::errno::consts::Errno) -> Self
+pub fn brush_core::error::ErrorKind::from(err: nix::errno::consts::Errno) -> Self
+pub mod brush_core::sys::commands
+pub use brush_core::sys::commands::CommandExt
+pub use brush_core::sys::commands::ExitStatusExt
+pub trait brush_core::sys::commands::CommandFdInjectionExt
+pub fn brush_core::sys::commands::CommandFdInjectionExt::inject_fds(&mut self, open_files: brush_core::openfiles::OpenFiles) -> core::result::Result<(), brush_core::error::Error>
+impl brush_core::sys::commands::CommandFdInjectionExt for std::process::Command
+pub fn std::process::Command::inject_fds(&mut self, open_files: brush_core::openfiles::OpenFiles) -> core::result::Result<(), brush_core::error::Error>
+pub trait brush_core::sys::commands::CommandFgControlExt
+pub fn brush_core::sys::commands::CommandFgControlExt::take_foreground(&mut self)
+impl brush_core::sys::commands::CommandFgControlExt for std::process::Command
+pub fn std::process::Command::take_foreground(&mut self)
+pub use brush_core::sys::fs::MetadataExt
+pub enum brush_core::sys::PlatformError
+pub brush_core::sys::PlatformError::ErrnoError(nix::errno::consts::Errno)
+pub brush_core::ErrorKind::NotSupportedOnThisPlatform(&'static str)
+pub brush_core::ErrorKind::PlatformError(brush_core::sys::PlatformError)

Performance Benchmark Report

Benchmark name Baseline (μs) Test/PR (μs) Delta (μs) Delta %
clone_shell_object 17.52 μs 18.29 μs 0.78 μs 🟠 +4.43%
eval_arithmetic 0.15 μs 0.15 μs 0.00 μs ⚪ Unchanged
expand_one_string 1.96 μs 1.99 μs 0.03 μs ⚪ Unchanged
for_loop 22.42 μs 22.62 μs 0.21 μs ⚪ Unchanged
function_call 2.58 μs 2.70 μs 0.12 μs ⚪ Unchanged
instantiate_shell 58.95 μs 58.73 μs -0.22 μs ⚪ Unchanged
instantiate_shell_with_init_scripts 27053.79 μs 24827.46 μs -2226.33 μs 🟢 -8.23%
parse_bash_completion 2081.23 μs 2050.60 μs -30.62 μs 🟢 -1.47%
parse_sample_script 2.01 μs 2.00 μs -0.01 μs ⚪ Unchanged
run_echo_builtin_command 15.96 μs 15.90 μs -0.07 μs ⚪ Unchanged
run_one_external_command 2378.30 μs 2368.11 μs -10.18 μs ⚪ Unchanged
tokenize_sample_script 3.48 μs 3.52 μs 0.03 μs ⚪ Unchanged

Code Coverage Report: Only Changed Files listed

Package Base Coverage New Coverage Difference
brush-core/src/commands.rs 🟢 87.36% 🟢 88.83% 🟢 1.47%
brush-core/src/jobs.rs 🔴 47.71% 🔴 43.58% 🔴 -4.13%
brush-core/src/openfiles.rs 🟠 64.74% 🟠 68.71% 🟢 3.97%
brush-core/src/sys/unix.rs 🔴 0% 🟢 100% 🟢 100%
brush-core/src/sys/unix/commands.rs 🔴 0% 🟠 57.14% 🟢 57.14%
brush-core/src/sys/unix/users.rs 🔴 46.34% 🟠 53.19% 🟢 6.85%
brush-core/src/traps.rs 🟢 90.67% 🟢 90.54% 🔴 -0.13%
brush-core/src/wellknownvars.rs 🟢 87.38% 🟢 86.97% 🔴 -0.41%
brush-shell/src/main.rs 🟢 87.39% 🟢 85.09% 🔴 -2.3%
Overall Coverage 🟢 73.27% 🟢 73.23% 🔴 -0.04%

Minimum allowed coverage is 70%, this run produced 73.23%
%`

Test Summary: bash-completion test suite

Outcome Count Percentage
✅ Pass 1345 63.77
❗️ Error 217 10.29
❌ Fail 178 8.44
⏩ Skip 354 16.79
❎ Expected Fail 14 0.66
✔️ Unexpected Pass 1 0.05
📊 Total 2109 100.00

@github-actions
Copy link
Copy Markdown

github-actions Bot commented Nov 6, 2025

Test Results

    3 files     23 suites   6m 48s ⏱️
1 080 tests 1 080 ✅ 0 💤 0 ❌
3 220 runs  3 220 ✅ 0 💤 0 ❌

Results for commit 618d01e.

♻️ This comment has been updated with latest results.

@reubeno reubeno requested a review from Copilot November 6, 2025 14:14
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR refactors platform-specific code to improve cross-platform consistency and maintainability. The changes abstract platform-specific functionality behind a unified interface, reducing conditional compilation directives throughout the codebase.

Key changes:

  • Introduced platform-specific commands module with unified extension traits (CommandExt, ExitStatusExt, CommandFdInjectionExt, CommandFgControlExt)
  • Consolidated signal handling through sys::signal::Signal abstraction
  • Replaced direct uzers and nix usage with sys::users abstraction
  • Added PlatformError type to unify platform-specific error handling
  • Created completion feature flag in brush-interactive to gate completion functionality
  • Standardized platform conditionals to use any(unix, windows) ordering consistently
  • Replaced error::unimp() calls with NotSupportedOnThisPlatform error variant
  • Removed procfs dependency and cleaned up unused imports

Reviewed Changes

Copilot reviewed 32 out of 33 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
brush-shell/src/shell_factory.rs Standardized platform conditionals order and added unused_variables allow for cross-platform compatibility
brush-shell/src/main.rs Extracted terminal reset logic to function, added unused_imports allow, converted exit code cast to use From trait
brush-shell/Cargo.toml Reordered platform conditionals for consistency
brush-interactive/src/lib.rs Changed completion module gate from platform-specific to feature-based
brush-interactive/Cargo.toml Added completion feature flag and made it a dependency of basic and reedline features
brush-core/src/wellknownvars.rs Replaced platform-specific code with sys abstractions for UID/EUID/HOSTTYPE
brush-core/src/traps.rs Abstracted signal handling through sys::signal::Signal
brush-core/src/sys/windows/users.rs Added get_current_uid/gid functions, updated error handling to use NotSupportedOnThisPlatform
brush-core/src/sys/windows.rs Added commands module and PlatformError type
brush-core/src/sys/wasm.rs Added commands module and PlatformError type
brush-core/src/sys/unix/users.rs Added get_current_uid/gid wrapper functions
brush-core/src/sys/unix/signal.rs Exported nix::sys::signal::Signal
brush-core/src/sys/unix/fs.rs Made MetadataExt public export instead of private use
brush-core/src/sys/unix/commands.rs New module with Unix-specific command extension trait implementations
brush-core/src/sys/unix.rs Added commands module and PlatformError type with From impl for nix::errno::Errno
brush-core/src/sys/stubs/users.rs Added get_current_uid/gid, updated error handling
brush-core/src/sys/stubs/signal.rs Added stub Signal enum with required methods
brush-core/src/sys/stubs/input.rs Updated error handling to use NotSupportedOnThisPlatform
brush-core/src/sys/stubs/fs.rs Renamed trait from StubMetadataExt to MetadataExt, updated error handling
brush-core/src/sys/stubs/commands.rs New module with stub command extension trait implementations
brush-core/src/sys/stubs.rs Added commands module
brush-core/src/sys/fs.rs Removed conditional MetadataExt re-exports (now handled in platform modules)
brush-core/src/sys.rs Added public exports for commands and PlatformError
brush-core/src/openfiles.rs Removed top-level Unix imports, moved to function scope, removed unused as_raw_fd method
brush-core/src/lib.rs Reordered module declarations (moved completion module down in list)
brush-core/src/interp.rs Replaced Unix-specific imports with sys::commands::ExitStatusExt, moved AsFd import to function scope
brush-core/src/error.rs Replaced platform-specific error variants with unified PlatformError variant, added NotSupportedOnThisPlatform variant
brush-core/src/commands.rs Replaced inline platform-specific code with sys::commands trait methods
brush-core/Cargo.toml Reordered platform conditionals, removed procfs dependency, added sync feature to wasm tokio
brush-builtins/src/read.rs Converted cast to use From trait
brush-builtins/src/factory.rs Added unused_imports allow for cross-platform compatibility
brush-builtins/Cargo.toml Reordered platform conditionals
Cargo.lock Removed procfs dependency

Comment thread brush-core/src/commands.rs Outdated
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@reubeno reubeno merged commit ac1c89d into main Nov 6, 2025
42 checks passed
@reubeno reubeno deleted the shuffle-cfg-code branch November 6, 2025 15:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants