Skip to content

Remove winnow from gix-actor public API#2546

Merged
Sebastian Thiel (Byron) merged 12 commits intomainfrom
fix-2545
Apr 27, 2026
Merged

Remove winnow from gix-actor public API#2546
Sebastian Thiel (Byron) merged 12 commits intomainfrom
fix-2545

Conversation

@Byron
Copy link
Copy Markdown
Member

@Byron Sebastian Thiel (Byron) commented Apr 26, 2026

It's too easy to not detect winnow updates as a breaking change, and then make patch releases that break existing builds.
More importantly, while at it, remove the notion of parser combinators as I found them hard to maintain, and return to simple imperative cursor-oriented parsers.

Tasks

Benches

It turns out that removing winnow leads to a simpler but faster parser implementation, and it's probably one that I can maintain as well.

CommitRef(sig)          time:   [573.77 ns 575.62 ns 577.64 ns]
                        change: [−1.8410% −1.3568% −0.9145%] (p = 0.00 < 0.05)
                        Change within noise threshold.
Found 5 outliers among 100 measurements (5.00%)
  5 (5.00%) high mild

CommitRefIter(sig)      time:   [603.76 ns 605.74 ns 607.76 ns]
                        change: [−7.0691% −6.7265% −6.3744%] (p = 0.00 < 0.05)
                        Performance has improved.

TagRef(sig)             time:   [51.190 ns 51.333 ns 51.499 ns]
                        change: [−41.190% −40.928% −40.650%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 1 outliers among 100 measurements (1.00%)
  1 (1.00%) low mild

TagRefIter(sig)         time:   [71.008 ns 71.387 ns 71.779 ns]
                        change: [−38.161% −37.832% −37.484%] (p = 0.00 < 0.05)
                        Performance has improved.

TreeRef()               time:   [59.948 ns 60.157 ns 60.390 ns]
                        change: [−2.1089% −1.6804% −1.2364%] (p = 0.00 < 0.05)
                        Performance has improved.

TreeRefIter()           time:   [26.899 ns 27.041 ns 27.207 ns]
                        change: [−2.7957% −1.1482% +0.2270%] (p = 0.16 > 0.05)
                        No change in performance detected.
Found 2 outliers among 100 measurements (2.00%)
  2 (2.00%) high mild

gix-config

promising as well

GitConfig large config file
                        time:   [18.956 µs 19.023 µs 19.092 µs]
                        change: [−13.068% −12.417% −11.835%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 1 outliers among 100 measurements (1.00%)
  1 (1.00%) high severe

Parser large config file
                        time:   [17.319 µs 17.377 µs 17.436 µs]
                        change: [−41.547% −31.527% −20.769%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 7 outliers among 100 measurements (7.00%)
  5 (5.00%) high mild
  2 (2.00%) high severe

Sebastian Thiel (Byron) and others added 2 commits April 27, 2026 11:37
It's replaced  with `BStrSlice::find_byte` which is the same
under the hood.
)

It's too easy to not detect `winnow` updates as a breaking change,
and then make patch releases that break existing builds.

Co-authored-by: Sebastian Thiel <sebastian.thiel@icloud.com>
…erywhere.

This will allow for simplified maintenance and editing (both human and machine)
down the road, and enable additional performance optimisations.

Parser compbinators to me ultimately were a failed experiment as I couldn't maintain
them anyway, with it being too difficult for me to grasp and express everything
in its very own kind of language, with a lot of different things to consider.

Note that this also removes detailed errors from all parsers that previously
used `winnow`, with the option to re-add those if there is demand.

Co-authored-by: Sebastian Thiel <sebastian.thiel@icloud.com>
This improves Git conformity, which is also pretty lenient.

Co-authored-by: Sebastian Thiel <sebastian.thiel@icloud.com>
@Byron Sebastian Thiel (Byron) marked this pull request as ready for review April 27, 2026 06:52
Copilot AI review requested due to automatic review settings April 27, 2026 06:52
Copy link
Copy Markdown

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 removes winnow from the public API surface (notably gix-actor) and replaces parser-combinator based parsing with imperative, cursor-oriented parsers across multiple crates. As part of the refactor, object parsing APIs are updated to require an explicit gix_hash::Kind so commit/tag parsing can validate SHA-1 vs SHA-256 object-id sizes correctly.

Changes:

  • Replace winnow-based parsers with manual cursor parsers in gix-actor, gix-object, gix-ref, gix-protocol, and gix-config, and remove winnow dependencies.
  • Thread gix_hash::Kind into commit/tag parsing entry points (and update call sites across crates).
  • Update tests/fuzz/benches and remove the former “verbose parsing errors” feature plumbing.

Reviewed changes

Copilot reviewed 76 out of 77 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
tests/tools/src/lib.rs Removes test helper tied to winnow error types.
tests/tools/Cargo.toml Drops winnow dependency from test tools crate.
justfile Removes verbose-object-parsing-errors checks and updates gix-object test invocations.
gix/src/object/tag.rs Passes object hash kind into gix_object tag parsing APIs.
gix/src/object/commit.rs Passes object hash kind into gix_object commit parsing APIs.
gix/Cargo.toml Removes verbose-object-parsing-errors feature wiring from gix.
gix-traverse/src/commit/simple.rs Tracks current object hash kind to parse buffered commits correctly.
gix-revwalk/src/graph/mod.rs Stores hash kind in LazyCommit to parse ODB-backed commits correctly.
gix-revwalk/src/graph/commit.rs Uses stored hash kind when parsing commit bytes.
gix-ref/tests/refs/file/reference.rs Adds coverage for uppercase hex ids in ref parsing.
gix-ref/src/store/packed/transaction.rs Threads hash_kind into tag peeling when resolving packed refs.
gix-ref/src/store/packed/iter.rs Replaces winnow-based packed-refs parsing with imperative parsing.
gix-ref/src/store/packed/find.rs Replaces winnow parsing in packed-refs lookup with imperative parsing.
gix-ref/src/store/packed/decode/tests.rs Updates packed-refs decode tests to new imperative parsing APIs and adds uppercase coverage.
gix-ref/src/store/packed/decode.rs Reimplements packed-refs header/reference decoding without winnow.
gix-ref/src/store/packed/buffer.rs Adjusts packed-refs header parsing and offset computation without winnow.
gix-ref/src/store/file/raw_ext.rs Threads hash_kind into tag peeling for loose refs.
gix-ref/src/store/file/loose/reference/decode.rs Replaces winnow parsing for loose refs with an imperative parser.
gix-ref/src/store/file/log/line.rs Reimplements reflog line parsing without winnow.
gix-ref/src/parse.rs Introduces shared imperative helpers for parsing hex ids and newlines.
gix-ref/Cargo.toml Drops winnow dependency from gix-ref.
gix-protocol/src/remote_progress.rs Reimplements remote progress parsing without winnow.
gix-protocol/Cargo.toml Drops winnow dependency from gix-protocol.
gix-path/tests/path/realpath.rs Adjusts fuzzed timeout threshold on Windows.
gix-pack/src/data/output/count/objects/mod.rs Passes hash_kind into commit/tag iterators while expanding objects.
gix-object/tests/object/tree/from_bytes.rs Simplifies invalid-tree assertions after error-type change.
gix-object/tests/object/tag.rs Updates tag tests for explicit hash kind; adds sha256/uppercase/PGP edge cases.
gix-object/tests/object/main.rs Adds fixture rewriting helpers for commit/tag in sha256 test runs.
gix-object/tests/object/encode.rs Updates round-trip tests to use hash-kind aware fixture loading and parsing.
gix-object/tests/object/commit/mod.rs Updates commit tests for explicit hash kind and simplified error assertions.
gix-object/tests/object/commit/iter.rs Updates commit iterator tests for explicit hash kind and signature API change.
gix-object/tests/object/commit/from_bytes.rs Adds sha256/uppercase coverage; updates parsing calls to pass hash kind.
gix-object/src/tree/ref_iter.rs Removes winnow error plumbing from tree iteration error reporting.
gix-object/src/tag/ref_iter.rs Updates tag iterator API to require hash kind; removes winnow dependency.
gix-object/src/tag/mod.rs Updates TagRef::from_bytes() signature to require hash kind.
gix-object/src/tag/decode.rs Reimplements tag parsing + PGP signature detection without winnow.
gix-object/src/parse.rs Reimplements shared object parsing utilities without winnow.
gix-object/src/object/mod.rs Threads hash_kind into commit/tag object parsing in ObjectRef.
gix-object/src/lib.rs Updates ref iterators to carry hash kind; simplifies decode error type.
gix-object/src/data.rs Threads hash_kind into commit/tag parsing + iter creation in Data.
gix-object/src/commit/ref_iter.rs Updates commit iterator API to require hash kind; removes winnow dependency.
gix-object/src/commit/mod.rs Updates CommitRef::from_bytes() signature to require hash kind; threads kind into mergetag parsing.
gix-object/src/commit/message/mod.rs Renames message decode entry point used by MessageRef.
gix-object/src/commit/message/decode.rs Reimplements title/body splitting without winnow.
gix-object/src/commit/decode.rs Reimplements full commit parsing + message parsing without winnow.
gix-object/fuzz/fuzz_targets/fuzz_tag.rs Runs tag fuzz parsing for both SHA-1 and SHA-256.
gix-object/fuzz/fuzz_targets/fuzz_commit.rs Runs commit fuzz parsing for both SHA-1 and SHA-256.
gix-object/benches/decode_objects.rs Updates benches to pass explicit hash kind into parsing APIs.
gix-object/Cargo.toml Removes winnow dep and removes verbose-object-parsing-errors feature.
gix-mailmap/src/lib.rs Updates docs/examples to new SignatureRef::from_bytes() signature.
gix-mailmap/fuzz/fuzz_targets/mailmap.rs Updates fuzz target to new SignatureRef::from_bytes() signature.
gix-imara-diff/src/sources.rs Removes memchr usage by switching to bstr::ByteSlice::find_byte.
gix-imara-diff/Cargo.toml Drops memchr dependency.
gix-config/tests/config/file/access/raw/set_existing_raw_value.rs Adds test for global properties using empty section name.
gix-config/tests/config/file/access/raw/raw_value.rs Adds test for global properties using empty section name.
gix-config/src/types.rs Updates docs to reflect that git accepts global properties before first section.
gix-config/src/parse/tests.rs Adds tests for edge-case section headers and event writing behavior.
gix-config/src/parse/nom/mod.rs Removes the prior winnow-based config parser implementation.
gix-config/src/parse/mod.rs Switches parser entry point to the new from_bytes module.
gix-config/src/parse/from_bytes/tests.rs Adds extensive tests for the new imperative config parser.
gix-config/src/parse/from_bytes/mod.rs New imperative, cursor-oriented config parser implementation.
gix-config/src/parse/events.rs Updates docs to match new parser behavior for global properties.
gix-config/src/lib.rs Adjusts crate-level deny lints after parser refactor.
gix-config/src/file/mutable/mod.rs Makes escape_value() pub(crate) for broader internal use.
gix-config/Cargo.toml Drops winnow and memchr dependencies.
gix-actor/tests/actor/signature.rs Updates tests to new SignatureRef::from_bytes() signature/error type.
gix-actor/tests/actor/identity.rs Updates tests to new IdentityRef::from_bytes() signature/error type.
gix-actor/src/signature/mod.rs Changes public parsing API to return gix_error::ValidationError; adds consuming parser.
gix-actor/src/signature/decode.rs Reimplements signature parsing without winnow and updates error handling.
gix-actor/src/lib.rs Updates docs/examples to new signature/identity parsing API.
gix-actor/src/identity.rs Changes public parsing API to return gix_error::ValidationError; adds consuming parser.
gix-actor/fuzz/fuzz_targets/actors.rs Updates fuzz target to new parsing APIs.
gix-actor/Cargo.toml Drops winnow dependency from gix-actor.
gitoxide-core/src/repository/mailmap.rs Updates identity parsing calls to new API.
gitoxide-core/src/query/engine/update.rs Threads hash_kind into commit parsing when extracting parents.
gitoxide-core/src/hours/mod.rs Threads hash_kind into commit parsing for trailer/author extraction.
Cargo.lock Removes winnow (and memchr where applicable) from the lockfile dependency graph.

Comment thread justfile
Comment thread gix-config/src/lib.rs
Comment thread gix-ref/src/parse.rs Outdated
Comment thread gix-ref/src/store/file/loose/reference/decode.rs
Doing so adds conformity with Git, but also simplifies the parser
which now only parse hex-hashes of a single valid length.

Co-authored-by: Sebastian Thiel <sebastian.thiel@icloud.com>
Co-authored-by: Sebastian Thiel <sebastian.thiel@icloud.com>
Copy link
Copy Markdown

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

Copilot reviewed 76 out of 77 changed files in this pull request and generated 1 comment.

Comment thread gix-ref/src/store/packed/decode.rs Outdated
Copy link
Copy Markdown

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

Copilot reviewed 106 out of 123 changed files in this pull request and generated 3 comments.

Comment thread gix-ref/src/store/file/loose/reference/decode.rs
Comment thread gix-ref/src/parse.rs Outdated
Comment thread gix-ref/fuzz/fuzz_targets/fuzz_packed_buffer.rs
Codex (codex) and others added 2 commits April 27, 2026 23:46
This predominantly restricts parsing so it won't allow any hash
but the one that was passed.

Co-authored-by: Sebastian Thiel <sebastian.thiel@icloud.com>
Co-authored-by: Sebastian Thiel <sebastian.thiel@icloud.com>
Mere convenience.

Co-authored-by: Sebastian Thiel <sebastian.thiel@icloud.com>
Copilot AI review requested due to automatic review settings April 27, 2026 22:04
Copy link
Copy Markdown

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

Copilot reviewed 109 out of 126 changed files in this pull request and generated no new comments.

… tests

- update some fixture so ci-tests at least pass locally
- maybe this removes one more source of flake?
Copilot AI review requested due to automatic review settings April 27, 2026 22:45
…th v0.12.0, gix-features v0.48.0, gix-hash v0.25.0, gix-hashtable v0.15.0, gix-object v0.60.0, gix-glob v0.26.0, gix-attributes v0.33.0, gix-command v0.9.0, gix-filter v0.30.0, gix-fs v0.21.0, gix-commitgraph v0.37.0, gix-revwalk v0.31.0, gix-traverse v0.57.0, gix-worktree-stream v0.32.0, gix-archive v0.32.0, gix-tempfile v23.0.0, gix-lock v23.0.0, gix-index v0.51.0, gix-config-value v0.18.0, gix-pathspec v0.18.0, gix-ignore v0.21.0, gix-worktree v0.52.0, gix-imara-diff v0.2.1, gix-diff v0.63.0, gix-blame v0.13.0, gix-ref v0.63.0, gix-sec v0.14.0, gix-config v0.56.0, gix-prompt v0.15.0, gix-url v0.36.0, gix-credentials v0.38.0, gix-discover v0.51.0, gix-dir v0.25.0, gix-mailmap v0.33.0, gix-revision v0.45.0, gix-merge v0.16.0, gix-negotiate v0.31.0, gix-pack v0.70.0, gix-odb v0.80.0, gix-refspec v0.41.0, gix-shallow v0.12.0, gix-transport v0.57.0, gix-protocol v0.61.0, gix-status v0.30.0, gix-submodule v0.30.0, gix-worktree-state v0.30.0, gix v0.83.0, gix-fsck v0.21.0, gitoxide-core v0.57.0, gitoxide v0.53.0, safety bump 48 crates

SAFETY BUMP: gix-features v0.48.0, gix-hash v0.25.0, gix-hashtable v0.15.0, gix-object v0.60.0, gix-glob v0.26.0, gix-attributes v0.33.0, gix-command v0.9.0, gix-filter v0.30.0, gix-fs v0.21.0, gix-commitgraph v0.37.0, gix-revwalk v0.31.0, gix-traverse v0.57.0, gix-worktree-stream v0.32.0, gix-archive v0.32.0, gix-tempfile v23.0.0, gix-lock v23.0.0, gix-index v0.51.0, gix-config-value v0.18.0, gix-pathspec v0.18.0, gix-ignore v0.21.0, gix-worktree v0.52.0, gix-diff v0.63.0, gix-blame v0.13.0, gix-ref v0.63.0, gix-sec v0.14.0, gix-config v0.56.0, gix-prompt v0.15.0, gix-url v0.36.0, gix-credentials v0.38.0, gix-discover v0.51.0, gix-dir v0.25.0, gix-mailmap v0.33.0, gix-revision v0.45.0, gix-merge v0.16.0, gix-negotiate v0.31.0, gix-pack v0.70.0, gix-odb v0.80.0, gix-refspec v0.41.0, gix-shallow v0.12.0, gix-transport v0.57.0, gix-protocol v0.61.0, gix-status v0.30.0, gix-submodule v0.30.0, gix-worktree-state v0.30.0, gix v0.83.0, gix-fsck v0.21.0, gitoxide-core v0.57.0, gitoxide v0.53.0
Copy link
Copy Markdown

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

Copilot reviewed 112 out of 131 changed files in this pull request and generated 1 comment.

Comment thread justfile
bad I and Codex keep forgetting to run it.
Copilot AI review requested due to automatic review settings April 27, 2026 23:08
Copy link
Copy Markdown

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

Copilot reviewed 199 out of 218 changed files in this pull request and generated 1 comment.

Comment thread tests/journey/gix.sh
@Byron Sebastian Thiel (Byron) merged commit adb8328 into main Apr 27, 2026
36 checks passed
@Byron Sebastian Thiel (Byron) deleted the fix-2545 branch April 27, 2026 23:36
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.

Unable to add any repository with fresh install of GitButler on MacOS - Unexpected token error Beta version of cargo (as a library) is broken by gix

3 participants