Skip to content

UPSTREAM PR #2423: gix-error conversion batch 1#20

Open
loci-dev wants to merge 12 commits into
mainfrom
loci/pr-2423-gix-error
Open

UPSTREAM PR #2423: gix-error conversion batch 1#20
loci-dev wants to merge 12 commits into
mainfrom
loci/pr-2423-gix-error

Conversation

@loci-dev
Copy link
Copy Markdown

@loci-dev loci-dev commented Feb 8, 2026

Note

Source pull request: GitoxideLabs/gitoxide#2423

Batch 1 Migration Complete

11 crates migrated from thiserror to gix-error, in 11 commits:
┌────────────────┬───────────────────────────────────────────────────────┬─────────────────────────────────────────────────────────────────────────┐
│     Crate      │                 Error types replaced                  │                            Downstream fixes                             │
├────────────────┼───────────────────────────────────────────────────────┼─────────────────────────────────────────────────────────────────────────┤
│ gix-bitmap     │ 1 (ewah::decode::Error)                               │ none                                                                    │
├────────────────┼───────────────────────────────────────────────────────┼─────────────────────────────────────────────────────────────────────────┤
│ gix-quote      │ 1 (ansi_c::undo::Error)                               │ none                                                                    │
├────────────────┼───────────────────────────────────────────────────────┼─────────────────────────────────────────────────────────────────────────┤
│ gix-mailmap    │ 1 (parse::Error)                                      │ none                                                                    │
├────────────────┼───────────────────────────────────────────────────────┼─────────────────────────────────────────────────────────────────────────┤
│ gix-fs         │ 1 (to_normal_path_components::Error)                  │ none                                                                    │
├────────────────┼───────────────────────────────────────────────────────┼─────────────────────────────────────────────────────────────────────────┤
│ gix-lock       │ 1 (acquire::Error)                                    │ gix-testtools                                                           │
├────────────────┼───────────────────────────────────────────────────────┼─────────────────────────────────────────────────────────────────────────┤
│ gix-path       │ 2 (relative_path::Error, realpath::Error)             │ gix-ref                                                                 │
├────────────────┼───────────────────────────────────────────────────────┼─────────────────────────────────────────────────────────────────────────┤
│ gix-packetline │ 3 (encode::Error, decode::Error, decode::band::Error) │ none                                                                    │
├────────────────┼───────────────────────────────────────────────────────┼─────────────────────────────────────────────────────────────────────────┤
│ gix-attributes │ 2 (name::Error, parse::Error)                         │ gix-pathspec                                                            │
├────────────────┼───────────────────────────────────────────────────────┼─────────────────────────────────────────────────────────────────────────┤
│ gix-url        │ 3 (parse::Error, UrlParseError, expand_path::Error)   │ gix-transport, gix                                                      │
├────────────────┼───────────────────────────────────────────────────────┼─────────────────────────────────────────────────────────────────────────┤
│ gix-hash       │ 7 (all error types)                                   │ gix-object, gix-pack, gix-odb, gix-index, gix-status, gix-protocol, gix │
├────────────────┼───────────────────────────────────────────────────────┼─────────────────────────────────────────────────────────────────────────┤
│ gix-features   │ 3 (DecompressError, CompressError, inflate::Error)    │ (included in gix-hash downstream)                                       │
└────────────────┴───────────────────────────────────────────────────────┴─────────────────────────────────────────────────────────────────────────┘
Verification: cargo check -p gix passes, all 21 test suites across the 11 crates pass with 0 failures.

Tasks

  • fixup commits with correct authorship
  • thorough review

Byron and others added 12 commits February 7, 2026 11:53
Replace the thiserror-derived `ewah::decode::Error` enum with
`gix_error::Exn<gix_error::Message>`, using `ok_or_raise()` for
Option-based error creation.

Co-Authored-By: Sebastian Thiel <[email protected]>
Replace the thiserror-derived `ansi_c::undo::Error` enum with
`gix_error::Exn<gix_error::Message>`, converting the `Error::new()`
factory and variant constructors to `message!()` calls.

Co-Authored-By: Sebastian Thiel <[email protected]>
Replace the thiserror-derived `parse::Error` enum with
`gix_error::Exn<gix_error::Message>`. Tests converted from
variant pattern matching to string-based assertions.

Co-Authored-By: Sebastian Thiel <[email protected]>
Replace the thiserror-derived `to_normal_path_components::Error`
enum with `gix_error::Exn<gix_error::Message>`.

Co-Authored-By: Sebastian Thiel <[email protected]>
Replace the thiserror-derived `acquire::Error` enum with
`gix_error::Exn<gix_error::Message>`. The manually-implemented
`commit::Error<T>` is unchanged.

Also fix gix-testtools for the new error type.

Co-Authored-By: Sebastian Thiel <[email protected]>
Replace the thiserror-derived `relative_path::Error` and
`realpath::Error` enums with `gix_error::Exn<gix_error::Message>`.

Also fix downstream gix-ref for the changed error type.

Co-Authored-By: Sebastian Thiel <[email protected]>
Replace the thiserror-derived `encode::Error`, `decode::Error`, and
`decode::band::Error` enums with `gix_error::Exn<gix_error::Message>`.
The manually-implemented `read::Error` is unchanged.

Co-Authored-By: Sebastian Thiel <[email protected]>
Replace the thiserror-derived `name::Error` struct and `parse::Error`
enum with `gix_error::Exn<gix_error::Message>`.

Also fix downstream gix-pathspec for the removed `attribute` field.

Co-Authored-By: Sebastian Thiel <[email protected]>
Replace the thiserror-derived `parse::Error`, `simple_url::UrlParseError`,
and `expand_path::Error` enums with `gix_error::Exn<gix_error::Message>`.

Also fix downstream gix-transport and gix for the changed error types.

Co-Authored-By: Sebastian Thiel <[email protected]>
Replace all 7 thiserror-derived error types (io::Error, oid::Error,
prefix::Error, prefix::from_hex::Error, decode::Error, hasher::Error,
verify::Error) with `gix_error::Exn<gix_error::Message>`.

Also fix downstream crates for the changed error types:
gix-object, gix-pack, gix-odb, gix-index, gix-status, gix-protocol, gix.

Co-Authored-By: Sebastian Thiel <[email protected]>
Replace the thiserror-derived `DecompressError`, `CompressError`, and
`inflate::Error` enums with `gix_error::Exn<gix_error::Message>`.
The dependency is conditional on the `zlib` feature.

Co-Authored-By: Sebastian Thiel <[email protected]>
- cargo fmt
- cargo clippy

Fix all downstream compilation and clippy issues caused by Batch 1
crates changing error types from thiserror enums to Exn<Message>:

- Add From<Infallible> for Exn<Message> in gix-error
- Fix test files that use ? into Box<dyn Error> with .into_error()
- Fix clippy redundant_closure_for_method_calls warnings
- Fix gitoxide-core and porcelain code for changed error types
- Add gix-error dev-dependencies where needed

Co-authored-by: Claude <[email protected]>
@loci-review
Copy link
Copy Markdown

loci-review Bot commented Feb 8, 2026

Overview

This analysis evaluates a systematic error handling migration from thiserror to gix-error across gitoxide, affecting 32,207 functions (2,741 modified, 6,095 new, 5,802 removed, 17,569 unchanged) in 163 files across 12 commits.

Binaries analyzed:

  • target.aarch64-unknown-linux-gnu.release.gix: -0.011% power consumption (negligible)
  • target.aarch64-unknown-linux-gnu.release.ein: +0.338% power consumption (minor increase)

The migration introduces measurable regressions in configuration access and error formatting paths, while preserving performance in hot paths (object access, pack operations, parallel processing).

Function Analysis

Configuration Access (ein binary):

  • boolean: +996,501ns response time (+4,169%), +95ns throughput (+42%). Called 7-8 times per status/checkout operation through fs_capabilities.
  • boolean_filter: +996,332ns response time (+4,823%), +0.01ns throughput (+0.002%). Thin wrapper showing downstream bottleneck.
  • path_filter: +996,334ns response time (+5,037%), -2.5ns throughput (-0.57%). Security-focused config access.

All three functions bottleneck at ~1ms response time, indicating a common downstream issue in section lookup or value parsing.

Path Operations (ein binary):

  • normalize: +996,337ns response time (+12,844%), -152ns throughput (-21%). Paradoxical profile with improved self-time but worse total time.
  • from_dot_git_dir: +1,992,654ns response time (+10,362%), -77ns throughput (-27%). Repository discovery initialization.
  • find_ceiling_height: +3,987,388ns response time (+8,739%), +0.9ns throughput (+0.4%). Ceiling directory matching.

These functions show improved self-execution but dramatically worse response times with unchanged source code, suggesting static analysis measurement artifacts rather than genuine regressions.

Error Handling (gix binary):

  • write_fmt: +592ns response time (+7,574%), 0ns throughput change. Error formatting overhead from gix-error's deeper call stack.
  • Box::fmt: +12,140ns response time (+324%), +129ns throughput (+2,640%). Recursive error tree traversal with string allocations.
  • mark_complete_and_common_ref: +642,237ns response time (+426%), +4,687ns throughput (+1,400%). Fetch negotiation with error handling overhead.

I/O Operations (gix binary):

  • write_all: +96ns response time (+1,224%), +81ns throughput (+1,031%). Standard library regression mitigated by BufWriter (64KB-512KB buffers).
  • default_read_buf: +9,055ns response time (+5,281%), +8ns throughput (+6.6%). Likely Rust toolchain difference.

Iterator Performance (gix binary):

  • gix_config::Iterator::next: +93,352ns response time (+6,845%), +6ns throughput (+2.5%). Severe regression suggesting compiler de-optimization of iterator chain.

Positive Changes:

  • gix_index::Debug::fmt (gix): -915ns response time (-69.6%). Error handling consolidation improved Debug trait optimization.
  • gix_index::Debug::fmt (ein): -295ns response time (-42.5%). Consistent improvement across binaries.

Other analyzed functions showed expected overhead in error destructors and trait implementations, or minimal changes in parallel dispatch logic.

Additional Findings

The migration represents an intentional architectural trade-off: accepting performance costs in cold paths (error display, configuration access) for improved error diagnostics with automatic call-site tracking, error trees, and reduced binary bloat. The ein binary's configuration access bottleneck (~1ms per call) represents the most significant performance concern warranting investigation. Standard library I/O regressions appear to stem from Rust toolchain differences rather than gitoxide changes, with existing buffering strategies providing effective mitigation. No GPU/ML operations were identified; gitoxide operates entirely in CPU and I/O domains.

🔎 Full breakdown: Loci Inspector.
💬 Questions? Tag @loci-dev.

@loci-dev loci-dev force-pushed the main branch 5 times, most recently from 4805387 to 853b34d Compare February 13, 2026 07:49
@loci-dev loci-dev force-pushed the main branch 4 times, most recently from dccca45 to 24496b3 Compare February 21, 2026 07:43
@loci-dev loci-dev force-pushed the main branch 4 times, most recently from f045646 to e746ced Compare February 27, 2026 07:49
@loci-dev loci-dev force-pushed the main branch 3 times, most recently from 3a9c4ae to a28c634 Compare March 10, 2026 07:47
@loci-dev loci-dev force-pushed the main branch 4 times, most recently from 3deba97 to 9b41e5f Compare March 18, 2026 07:49
@loci-dev loci-dev force-pushed the main branch 6 times, most recently from 95ef755 to a9e7940 Compare March 26, 2026 07:51
@loci-dev loci-dev force-pushed the main branch 5 times, most recently from 8b02847 to 1bf0519 Compare April 2, 2026 07:54
@loci-dev loci-dev force-pushed the main branch 3 times, most recently from cdbe120 to 78a7ab5 Compare April 11, 2026 07:49
@loci-dev loci-dev force-pushed the main branch 6 times, most recently from 49231d8 to bc0a777 Compare April 20, 2026 07:18
@loci-dev loci-dev force-pushed the main branch 4 times, most recently from e902b63 to c6739bc Compare April 27, 2026 07:26
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.

2 participants