Subject: Re: [PATCH v2 00/18] Introduce rust: In xdiff

From: Patrick Steinhardt <[email-masked]>
Date: Wed, 17 Sep 2025 07:58:36 +0200

On Wed, Sep 17, 2025 at 01:16:20AM +0000, Ezekiel Newren via GitGitGadget wrote:
> This is a continuation of
> https://lore.kernel.org/git/[email-masked]/,
> but I am removing the RFC label.
> 
> Suggestions on changes that I could make to this series is appreciated.
> 
> Changes in v2: High level overview:
> 
>  * patch 1: cleanup: rename variables that collide with Rust primitive type
>    names
>  * patches 2-4: Makefile now produces libgit.a correctly
>  * patches 5,6: Documentation from Brian and Patrick
>  * patches 7,8: Introduce Rust
>  * patches 9-11: github workflows
>  * patches 12-14: introduce crates build-helper, and cbindgen
>  * patches 15-18: varint test balloon
> 
> I would like feed back in two categories: big changes, and little changes.
> Is my patch series even going in the right direction, and what are the
> little details that I've missed? I know Brian asked for cbindgen and it made
> the series several commits longer, but I think it's a great idea to have. I
> am not happy with every choice that I made, but I currently don't see a
> better way or at least an easier alternative to my approach in bringing Rust
> to Git.
> 
> Build results for these changes:
> https://github.com/git/git/actions/runs/17783386212?pr=2043. Some of these
> are failing.

Given that this isn't yet ready and given that this patch series is way
bigger than the one I have in flight that focusses on introducing the
policy [1]: how about we first merge that one down and then rebase your
patches on top of it to iterate? It would reduce the scope of your patch
series and allow us to make smaller steps towards our goal.

To be clear: I very much think that most of the steps here are quite
sensible. We definitely want to introduce cbindgen, we obviously need to
introduce support on Windows, and I guess having multiple different
workspaces is also something that could provide benefit.

But there is no reason to have that all at once, so hence my suggestion
to build out the infra one step at a time.

What do you think?

Patrick

[1]: https://lore.kernel.org/git/[email-masked]/