Skip to content

Conversation

@Byron
Copy link
Member

@Byron Byron commented Jan 18, 2026

Make it possible to follow gix-error types with source() chains, and
provide a conversion to anyhow (beyind a feature toggle).

Tasks

  • anyhow conversion from Exn
  • source-chain for Exn
    • make it possible to keep the location.
  • source-chain for Error
  • A way to turn on auto-source-chain for Error so gix errors will be compatible automatically

@Byron Byron force-pushed the gix-error branch 3 times, most recently from cbf6002 to 680cfc9 Compare January 18, 2026 18:00
@Byron Byron marked this pull request as ready for review January 18, 2026 21:01
@Byron Byron requested a review from Copilot January 18, 2026 21:01
Copy link

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 pull request adds anyhow integration to gix-error and implements error source chains for the Exn and Error types. The main enhancement is the addition of an auto-chain-error feature flag that automatically flattens error trees into chains, making them compatible with anyhow and other error-handling libraries.

Changes:

  • Added new concrete error types (Message, ParseError, ChainedError) in gix-error
  • Implemented std::error::Error::source() chains for Exn and Error types
  • Added auto-chain-error feature toggle with tree-error override, making auto-chain-error the default
  • Refactored message!() macro usage throughout the codebase to use format string capture syntax
  • Added anyhow conversion support via the anyhow feature flag

Reviewed changes

Copilot reviewed 24 out of 27 changed files in this pull request and generated 16 comments.

Show a summary per file
File Description
justfile Added test execution for auto-chain-error feature
gix/src/revision/spec/parse/delegate/*.rs Updated message! macro calls to use format string capture syntax
gix/Cargo.toml Added auto-chain-error to default features, tree-error to dev features
gix-error/tests/error/*.rs Updated tests to work with Message type and new error APIs
gix-error/tests/auto_chain_error.rs New test file validating auto-chain-error behavior
gix-error/src/lib.rs Added feature documentation, write_location helper, and concrete error module
gix-error/src/exn/impls.rs Implemented anyhow conversion, ChainedError conversion, and renamed iter() to iter_frames()
gix-error/src/error.rs Split implementation based on feature flags for tree vs chain error handling
gix-error/src/concrete/*.rs New concrete error types (Message, ParseError, ChainedError)
gix-error/Cargo.toml Added feature flags and dependencies for anyhow integration
gix-commitgraph/src/*.rs Updated error message formatting to use capture syntax
gitoxide-core/src/*.rs Removed manual error conversions now handled automatically
gitoxide-core/Cargo.toml Added gix-error dependency with anyhow feature
Cargo.lock Updated dependency graph

@EliahKagan
Copy link
Member

The test-32bit test failures (on arm32v7) were:

        FAIL [   0.003s] (1196/2983) gix-error::auto-chain-error from_exn_error
  stdout ───

    running 1 test
    test from_exn_error ... FAILED

    failures:

    failures:
        from_exn_error

    test result: FAILED. 0 passed; 1 failed; 0 ignored; 0 measured; 2 filtered out; finished in 0.00s
    
  stderr ───

    thread 'from_exn_error' (19568) panicked at gix-error/tests/auto_chain_error.rs:7:5:
    assertion `left == right` failed
      left: "Message(\"one\")"
     right: "one"
    note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

        FAIL [   0.003s] (1197/2983) gix-error::auto-chain-error from_exn_error_tree
  stdout ───

    running 1 test
    test from_exn_error_tree ... FAILED

    failures:

    failures:
        from_exn_error_tree

    test result: FAILED. 0 passed; 1 failed; 0 ignored; 0 measured; 2 filtered out; finished in 0.00s
    
  stderr ───

    thread 'from_exn_error_tree' (19570) panicked at gix-error/tests/auto_chain_error.rs:20:5:
    assertion `left == right` failed
      left: "Message(\"topmost\")\n|\n└─ Message(\"E6\")\n    |\n    └─ Message(\"E5\")\n    |   |\n    |   └─ Message(\"E3\")\n    |   |   |\n    |   |   └─ Message(\"E1\")\n    |   |\n    |   └─ Message(\"E10\")\n    |   |   |\n    |   |   └─ Message(\"E9\")\n    |   |\n    |   └─ Message(\"E12\")\n    |       |\n    |       └─ Message(\"E11\")\n    |\n    └─ Message(\"E4\")\n    |   |\n    |   └─ Message(\"E2\")\n    |\n    └─ Message(\"E8\")\n        |\n        └─ Message(\"E7\")"
     right: "topmost"
    note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

Given that no tests fail in the test-fast jobs that have completed so far, I don't know why this would happen. The error seems somehow specific to 32-bit targets, but I don't know why that would be.

@Byron
Copy link
Member Author

Byron commented Jan 18, 2026

I don't think it is specific to 32bits, I hope it will resolve itself. In any case, I will see tomorrow 😅.

@EliahKagan
Copy link
Member

EliahKagan commented Jan 18, 2026

Sounds good! It seemed weird enough that I figured I'd mention it. (Also, sorry about not saying what commit, fd8c038, those CI results were from--a new commit was pushed around the same time as I posted that comment.)

But actually it wasn't as weird as I thought:

  • In ci.yml, it looks like some relevant tests were excluded here in the test-fast job. That might be sufficient to explain the combination of passing and failing jobs. If not...

  • The failures seem like the usually happened when GIX_TEST_IGNORE_ARCHIVES=1 is set: the text-fixtures-windows job also failed, in the same way, it just hadn't finished yet when I commented. This is set in test-32bit, too, but not in test-fast. This is to say that, other than the "full" test job, they always happened in the jobs that rerun all fixture scripts. I mention this because, if that turns out to be relevant, then it suggest there may be less than the intended amout of feature coverage--for example, some missing nextest commands--in the justfile recipes used by the test job.

In any case, more pass since the subsequent commit a435a29.

Edit: Those two tests still fail in test-fixtures-windows though.

Byron added 4 commits January 19, 2026 04:34
This way, the behaviour of `gix::Error` is the same as it was before
without actually exposing an `Exn` error tree.
@Byron
Copy link
Member Author

Byron commented Jan 19, 2026

Thanks @EliahKagan. I have to confess that I did something quite messy: there is gix-error with a positive and a negative feature toggle. The positive one turns on a feature which disturbs gix-error's own tests which expect it to be off. cargo nextest doesn't handle feature resolution correctly by building everything at once seemingly, so the normal nextest run now needs gix-error barred from running, whenever it is called.

Maybe this knowledge should be put into the justfile.

The origin of this issue is cargo test -p gix which must disable default features in gix, but instead now has to use that negative feature toggle to do it.

Maybe a better way to fix this is to change the gix-error tests to expect either this or that depending on how they are run, and, I think that would be a good follow-up. Keeping note of that.

@Byron Byron merged commit 9d39656 into main Jan 19, 2026
30 checks passed
@Byron Byron deleted the gix-error branch January 19, 2026 03:51
@aliali05340-tech

This comment was marked as off-topic.

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.

4 participants