Introduce rust: In xdiff #2043
Closed
Introduce rust: In xdiff #2043
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
Author
|
/submit |
|
Submitted as [email protected] To fetch this version into To fetch this version to local tag |
|
Which vector type should I use or could you explain to me the choices if I am responsible for making that choice and what does the functions do to where it could fit my needs
Yahoo Mail: Search, Organize, Conquer
On Fri, Aug 29, 2025 at 3:44 PM, Ezekiel ***@***.***> wrote:
ezekielnewren left a comment (git/git#2043)
/submit
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.Message ID: ***@***.***>
|
![gitgitgadget-git[bot]](https://avatars.githubusercontent.com/in/46807?s=60&v=4){kind=small}
| TECH_DOCS += technical/multi-pack-index | ||
| TECH_DOCS += technical/pack-heuristics | ||
| TECH_DOCS += technical/parallel-checkout | ||
| TECH_DOCS += technical/partial-clone |
There was a problem hiding this comment.
On the Git mailing list, "brian m. carlson" wrote (reply to this):
On 2025-08-29 at 19:42:05, brian m. carlson via GitGitGadget wrote:
> +Crates like libc or rustix define types like c_long, but in ways that are not
> +safe across platforms.
> +From https://docs.rs/rustix/latest/rustix/ffi/type.c_long.html:
> +
> + This type will always be i32 or i64. Most notably, many Linux-based
> + systems assume an i64, but Windows assumes i32. The C standard technically
> + only requires that this type be a signed integer that is at least 32 bits
> + and at least the size of an int, although in practice, no system would
> + have a long that is neither an i32 nor i64.
> +
> +Also, note that other locations, such as
> +https://docs.rs/libc/latest/libc/type.c_long.html, just hardcode c_long as i64
> +even though C may mean i32 on some platforms.
> +
> +As such, using the c_long type would give us portability issues, and
> +perpetuate some of the bugs git has faced across platforms. Avoid using C's
> +types (long, unsigned, char, etc.), and switch to unambiguous types (e.g. i32
> +or i64) before trying to make C and Rust interoperate.
This makes sense. I agree fixed-size types are better and less brittle.
> +Crates like libc and rustix may have also traditionally aided interoperability
> +with older versions of Rust (e.g. when worrying about stat[64] system calls),
> +but the Rust standard library in newer versions of Rust handle these concerns
> +in a platform agnostic way. There may arise cases where we need to consider
> +these crates, but for now we omit them.
I'm fine with omitting them for now. However, we may very well need
them in the future.
> +Tools like bindgen and cbindgen create C-styled unsafe Rust code rather than
> +idiomatic Rust; where possible, we prefer to switch to idiomatic Rust. Any
> +standard C library functions that are needed can be manually wrapped on the
> +Rust side.
I agree that we want to use idiomatic Rust whenever possible. However,
I don't want to define structures and function definitions in both
languages and rely on people keeping them in sync, since that's a great
way to create brittle, broken code. Very notably, I have seen these
kinds of misalignments break only on big-endian architectures, which
most of us do not use, so we can really end up causing problems that are
very subtle this way.
I would prefer we wrote these functions with cbindgen to avoid this. If
we define structures only in Rust and never ever use C structures, then
we can avoid bindgen.
--
brian m. carlson (they/them)
Toronto, Ontario, CAThere was a problem hiding this comment.
On the Git mailing list, Ezekiel Newren wrote (reply to this):
On Fri, Aug 29, 2025 at 2:00 PM brian m. carlson
<[email protected]> wrote:
> > +Tools like bindgen and cbindgen create C-styled unsafe Rust code rather than
> > +idiomatic Rust; where possible, we prefer to switch to idiomatic Rust. Any
> > +standard C library functions that are needed can be manually wrapped on the
> > +Rust side.
>
> I agree that we want to use idiomatic Rust whenever possible. However,
> I don't want to define structures and function definitions in both
> languages and rely on people keeping them in sync, since that's a great
> way to create brittle, broken code. Very notably, I have seen these
> kinds of misalignments break only on big-endian architectures, which
> most of us do not use, so we can really end up causing problems that are
> very subtle this way.
>
> I would prefer we wrote these functions with cbindgen to avoid this. If
> we define structures only in Rust and never ever use C structures, then
> we can avoid bindgen.
Could you create a patch with your preferred wording about bindgen and
cbindgen for me? I'd prefer to introduce bindgen/cbindgen in a
different patch series because this one is already doing a lot just to
make Rust exist in Git.There was a problem hiding this comment.
On the Git mailing list, "brian m. carlson" wrote (reply to this):
On 2025-08-29 at 20:11:46, Ezekiel Newren wrote:
> Could you create a patch with your preferred wording about bindgen and
> cbindgen for me? I'd prefer to introduce bindgen/cbindgen in a
> different patch series because this one is already doing a lot just to
> make Rust exist in Git.
I think it's fine to introduce it in a different series. I'll plan to
do that myself if it doesn't get done sooner.
My change to the text is the following:
Tools like bindgen and cbindgen create C-styled unsafe Rust code rather than
idiomatic Rust; where possible, we prefer to switch to idiomatic Rust.
However, we may use bindgen and cbindgen to share existing Git types as an
interim step.
You can find the patch at https://github.com/bk2204/git.git in the
`rust-policy` branch.
--
brian m. carlson (they/them)
Toronto, Ontario, CAThere was a problem hiding this comment.
On the Git mailing list, Ezekiel Newren wrote (reply to this):
On Tue, Sep 2, 2025 at 10:39 AM brian m. carlson
<[email protected]> wrote:
> I think it's fine to introduce it in a different series. I'll plan to
> do that myself if it doesn't get done sooner.
Actually now that I'm knee deep in adding cbindgen to Rust it'll make
less sense to add it later. I'm currently working on refactoring my
entire patch series to include cbindgen from the beginning. I haven't
looked into cbindgen until now because I wanted to understand at a
deep level how C <-> Rust ffi worked rather than using an automagical
tool like cbindgen. I now think cbindgen should be part of the
introduction of Rust.There was a problem hiding this comment.
On the Git mailing list, Ezekiel Newren wrote (reply to this):
On Tue, Sep 2, 2025 at 12:39 PM Ezekiel Newren <[email protected]> wrote:
> Actually now that I'm knee deep in adding cbindgen to Rust it'll make
> less sense to add it later. I'm currently working on refactoring my
> entire patch series to include cbindgen from the beginning. I haven't
> looked into cbindgen until now because I wanted to understand at a
> deep level how C <-> Rust ffi worked rather than using an automagical
> tool like cbindgen. I now think cbindgen should be part of the
> introduction of Rust.
I've been able to get cbindgen to work with 1.63.0, but I'm not ready
to release my next patch series yet.
|
User |
Contributor
Author
It's meant to be used as a stepping stone to convert existing C code into Rust. If you're not doing that, then you don't need it. For pure Rust use Vec. |
Member
@ezekielnewren please reply on-list (as opposed to a PR comment; There is a long-standing usability wart in GitGitGadget). |
|
This patch series was integrated into seen via c7d8b5d. |
|
This branch is now known as |
|
This patch series was integrated into seen via c7305c7. |
|
There was a status update in the "Cooking" section about the branch Rust! Comments? source: <[email protected]> |
|
This patch series was integrated into seen via bfcd291. |
|
There was a status update in the "Cooking" section about the branch Rust! Not Rust, though ;-) Comments? source: <[email protected]> |
ezekielnewren
force-pushed
the
introduce_rust
branch
from
9fdd23a to
f504521
Compare
|
There are issues in commit d5056cb: |
|
There are issues in commit 4a7b80e: |
|
There are issues in commit 9784f4d: |
|
There are issues in commit eac0d25: |
|
There are issues in commit a2e059e: |
|
There are issues in commit a2bb4dc: |
|
There are issues in commit 15ef617: |
|
There are issues in commit ecddb40: |
|
There are issues in commit 0842b8b: |
ezekielnewren
force-pushed
the
introduce_rust
branch
from
f504521 to
659d508
Compare
Use a regex to find and rename variables that collide with Rust
primitive integer and float type names:
git grep -n -E -e '\<([ui](8|16|32|64|size)|(f(32|64)))\>'
Matches were reviewed and renamed. The remaining matches don't count
because:
- Rust source files:
contrib/libgit-rs/src/config.rs
contrib/libgit-sys/src/lib.rs
t/t4018/rust-impl
t/t4018/rust-trait
- Intentional references:
t/helper/test-parse-options.c (prints Rust int names)
t/t0040-parse-options.sh (tests the above)
View with --color-words to highlight the variable renames.
Signed-off-by: Ezekiel Newren <[email protected]>
Signed-off-by: Ezekiel Newren <[email protected]>
Signed-off-by: Ezekiel Newren <[email protected]>
Signed-off-by: Ezekiel Newren <[email protected]>
| export PERL_PATH | ||
| export PYTHON_PATH | ||
|
|
||
| TEST_SHELL_PATH = $(SHELL_PATH) |
There was a problem hiding this comment.
On the Git mailing list, Junio C Hamano wrote (reply to this):
"Ezekiel Newren via GitGitGadget" <[email protected]> writes:
> From: Ezekiel Newren <[email protected]>
>
> Signed-off-by: Ezekiel Newren <[email protected]>
> ---
> Makefile | 39 ++++++++++++++++-----------------------
> 1 file changed, 16 insertions(+), 23 deletions(-)
Aside from the comment already given about the fact that the
proposed log message does not explain any reason why these change
are necessary, this step and the previous step are fairly hostile to
merging the topic to play well with other topics, especially given
that there would be topics in flight that may want to add, remove,
or reorder these two existing lists.
I wonder if these could have been arranged like the following instead?
* Drop "REFTABLE_LIB = reftable/libreftable.a" and the target that
runs "ar" to mantain that archive.
* Leave "REFTABLE_OBJS += $objects.o" lines alone.
* Add them into LIB_OBJS so that they are included in libgit.a,
perhaps a single line like this:
LIB_OBJS += $(REFTABLE_OBJS)
Wouldn't that have worked equally well for the (unstated) purpose of
these two patches without incurring unnecessary risk of mismerges?
Similar arrangement for xdiff.
Thanks.There was a problem hiding this comment.
On the Git mailing list, Ezekiel Newren wrote (reply to this):
On Fri, Sep 19, 2025 at 1:02 PM Junio C Hamano <[email protected]> wrote:
> Aside from the comment already given about the fact that the
> proposed log message does not explain any reason why these change
> are necessary, this step and the previous step are fairly hostile to
> merging the topic to play well with other topics, especially given
> that there would be topics in flight that may want to add, remove,
> or reorder these two existing lists.
>
> I wonder if these could have been arranged like the following instead?
>
> * Drop "REFTABLE_LIB = reftable/libreftable.a" and the target that
> runs "ar" to mantain that archive.
>
> * Leave "REFTABLE_OBJS += $objects.o" lines alone.
>
> * Add them into LIB_OBJS so that they are included in libgit.a,
> perhaps a single line like this:
>
> LIB_OBJS += $(REFTABLE_OBJS)
>
> Wouldn't that have worked equally well for the (unstated) purpose of
> these two patches without incurring unnecessary risk of mismerges?
>
> Similar arrangement for xdiff.
Like the previous two commits; This one continues the effort to get
the Rust compiler to link against libgit.a. Meson already includes the
reftable in its libgit.a, but Makefile does not.
The reason why I was trying to get the Rust compiler to link against
libgit.a is because I wanted to get Rusts unit testing to work. If the
Rust code calls a C function from Git then 'cargo test' needs to know
about it.
However I think I'll drop these 3 commits since 'cargo test' doesn't
need to be part of the introduction of Rust. It would be nice for make
to be able to run Rust unit tests at some point though.There was a problem hiding this comment.
On the Git mailing list, Junio C Hamano wrote (reply to this):
Ezekiel Newren <[email protected]> writes:
> On Fri, Sep 19, 2025 at 1:02 PM Junio C Hamano <[email protected]> wrote:
>> Aside from the comment already given about the fact that the
>> proposed log message does not explain any reason why these change
>> are necessary, this step and the previous step are fairly hostile to
>> merging the topic to play well with other topics, especially given
>> that there would be topics in flight that may want to add, remove,
>> or reorder these two existing lists.
>>
>> I wonder if these could have been arranged like the following instead?
>>
>> * Drop "REFTABLE_LIB = reftable/libreftable.a" and the target that
>> runs "ar" to mantain that archive.
>>
>> * Leave "REFTABLE_OBJS += $objects.o" lines alone.
>>
>> * Add them into LIB_OBJS so that they are included in libgit.a,
>> perhaps a single line like this:
>>
>> LIB_OBJS += $(REFTABLE_OBJS)
>>
>> Wouldn't that have worked equally well for the (unstated) purpose of
>> these two patches without incurring unnecessary risk of mismerges?
>>
>> Similar arrangement for xdiff.
>
> Like the previous two commits; This one continues the effort to get
> ...
> The reason why ...
Neither answers my main question, though.
Instead of rolling everything into LIB_OBJS directly, wouldn't it
have been much easier to work with if reftable-related ones are left
in REFTABLE_OBJS and then RERFTABLE_OBJS gets added to LIB_OBJS?
Wouldn't it have been less prone to mismerges to do it that way?
> However I think I'll drop these 3 commits since 'cargo test' doesn't
> need to be part of the introduction of Rust. It would be nice for make
> to be able to run Rust unit tests at some point though.
As we can always extend things more, getting something close to the
minimally viable set with some tests for sanity checking would be a
good first goal. If you pare down way too much, however, we may end
up to be pretty close to what Patrick sent out originally with the
varint conversion, so let's make sure we do not drop below the
minimum that still demonstrates that we have Rust integration that
is viable going forward.
Thanks.There was a problem hiding this comment.
On the Git mailing list, Ezekiel Newren wrote (reply to this):
On Fri, Sep 19, 2025 at 2:14 PM Junio C Hamano <[email protected]> wrote:
> Instead of rolling everything into LIB_OBJS directly, wouldn't it
> have been much easier to work with if reftable-related ones are left
> in REFTABLE_OBJS and then RERFTABLE_OBJS gets added to LIB_OBJS?
> Wouldn't it have been less prone to mismerges to do it that way?
I didn't do it that way because I didn't think of that. I prefer your
way because it's much cleaner.
> As we can always extend things more, getting something close to the
> minimally viable set with some tests for sanity checking would be a
> good first goal. If you pare down way too much, however, we may end
> up to be pretty close to what Patrick sent out originally with the
> varint conversion, so let's make sure we do not drop below the
> minimum that still demonstrates that we have Rust integration that
> is viable going forward.
I'm torn between A and B from [1]:
I'm thinking of choosing A because my approach and Patrick's approach
are incompatible. In order to use multiple crates and ensure that Rust
is built and tested the same way with Make and Meson I'd have to rip
out lots of things from Patrick's patch series.
But maybe I should go with B because there's a lot of prep work that
should be done with Git before I can start doing what I'd like to do
in Rust. So let Patrick's stuff merge while I work on cleaning up
xdiff and updating Makefile's libgit.a build process and then deal
with ugly merging later.
I can't decide between the two.
[1] https://lore.kernel.org/git/CABPp-BHJUkSERQon6xx=sHrhN7i=6ekv+Hz1+P+2mh0=[email protected]/
|
This patch series was integrated into seen via 6793414. |
|
|
||
| EXTLIBS = | ||
|
|
||
| GIT_BUILD_DIR := $(CURDIR) |
There was a problem hiding this comment.
On the Git mailing list, Ezekiel Newren wrote (reply to this):
On Tue, Sep 16, 2025 at 7:17 PM Ezekiel Newren via GitGitGadget
<[email protected]> wrote:
To: Eric Sunshine
It looks like my gmail never received your email, so I'll respond to
my own patch.
> Please extend the commit message to at least give _some_ information
> about what the "misc" crate is all about since it is not at all
> apparent based upon the name.
>
> By the way, is "misc" really a good name? It sounds like it's going to
> be a dumping ground for anything which doesn't fit anywhere else.
I don't like the name misc either. What should we call the crate that
will be the new home for .c and .h files that live in the root of the
Git repository? varint is so tiny that creating a crate just for it
seems unjustified.
> Do I understand correctly from the comment that this crate has
> something to do with "xdiff", yet there doesn't actually seem to be
> anything here referencing "xdiff"? Am I missing something?
This crate has nothing to do with xdiff. I copy pasted the Cargo.toml
file from the xdiff crate and forgot to refactor the comment.There was a problem hiding this comment.
On the Git mailing list, Eric Sunshine wrote (reply to this):
On Fri, Sep 19, 2025 at 4:42 PM Ezekiel Newren <[email protected]> wrote:
> On Tue, Sep 16, 2025 at 7:17 PM Ezekiel Newren via GitGitGadget
> <[email protected]> wrote:
> To: Eric Sunshine
> It looks like my gmail never received your email, so I'll respond to
> my own patch.
Indeed, by sending reviews about earlier patches in this series, it
somehow triggered Gmail to think that I was spamming you, hence Gmail
thenceforth refused to deliver all subsequent emails to you (and
several other Gmail addresses in the recipient list, including Ben and
GitGitGadget). Hence, you would (presumably) have only received my
review emails and other responses indirectly via the Git mailing list
itself (assuming you are subscribed). As far as I know, Gmail still
considers my email address as being a spammer to you, so I'm using a
different email address to respond here.
> > Please extend the commit message to at least give _some_ information
> > about what the "misc" crate is all about since it is not at all
> > apparent based upon the name.
> >
> > By the way, is "misc" really a good name? It sounds like it's going to
> > be a dumping ground for anything which doesn't fit anywhere else.
>
> I don't like the name misc either. What should we call the crate that
> will be the new home for .c and .h files that live in the root of the
> Git repository? varint is so tiny that creating a crate just for it
> seems unjustified.
Would the name `gitcore` or `git-core` be suitable?There was a problem hiding this comment.
On the Git mailing list, Ezekiel Newren wrote (reply to this):
On Fri, Sep 19, 2025 at 2:50 PM Eric Sunshine <[email protected]> wrote:
> Would the name `gitcore` or `git-core` be suitable?
The only other name I could think of was `util`, but I think I prefer
`gitcore`. Does anybody else have any name suggestions?
|
On the Git mailing list, Ezekiel Newren wrote (reply to this): On Tue, Sep 16, 2025 at 11:58 PM Patrick Steinhardt <[email protected]> wrote:
> 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?
I think I made a big mistake of not making it clear that I intended v2
as more of an RFC. My worry (that I expressed very poorly or not at
all) is how hard will it be to apply my patches on top of your
foundation? I don't know if minor or major changes to your current
path would be needed, so I proposed a different way to Introduce Rust
while trying to incorporate work from others.
I wanted feedback on:
* Cleaning up Rust type name collisions
* People don't like it, so I'll drop that
* Have Makefile produce libgit.a correctly.
* I think this is a good idea, but it doesn't belong in this patch series.
* Adding Rust unit test which required fixing Make and adding
build-helper to reduce boilerplate code
* I think this is a good idea, but should be added later.
* Introduce cbindgen to avoid desync errors between Rust and C.
* LIke other points above; This should be added later.
* What should we call the crate that will be the new home for .c and
.h files in the root of Git? |
|
There was a status update in the "New Topics" section about the branch Expecting a reroll. source: <[email protected]> |
|
User |
|
User |
|
This patch series was integrated into seen via c7b70e8. |
|
On the Git mailing list, Patrick Steinhardt wrote (reply to this): On Fri, Sep 19, 2025 at 02:57:58PM -0600, Ezekiel Newren wrote:
> On Tue, Sep 16, 2025 at 11:58 PM Patrick Steinhardt <[email protected]> wrote:
> > 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?
>
> I think I made a big mistake of not making it clear that I intended v2
> as more of an RFC. My worry (that I expressed very poorly or not at
> all) is how hard will it be to apply my patches on top of your
> foundation? I don't know if minor or major changes to your current
> path would be needed, so I proposed a different way to Introduce Rust
> while trying to incorporate work from others.
Ah, fair enough. Thanks for clarifying your intentions!
I don't think there'd have to be major changes to the current version
of my patch series. The idea of that patch series is very much getting
buy-in regarding our roadmap and focus less on the actual build infra.
So the changes introduced are mostly an MVC, and I very much think that
we'll have to iterate quite a bit on it, but that's intended.
I think that a lot of the steps you outline below are logical next steps
to get there. If I can change anything to make these next steps easier
for you I'm happy to do so. But I also don't think it's too bad if we
have to change the current infra quite significantly to get there.
> I wanted feedback on:
> * Cleaning up Rust type name collisions
> * People don't like it, so I'll drop that
I don't have a strong opinion on this. If it creates issues I personally
don't mind fixing it.
> * Have Makefile produce libgit.a correctly.
> * I think this is a good idea, but it doesn't belong in this patch series.
> * Adding Rust unit test which required fixing Make and adding
> build-helper to reduce boilerplate code
> * I think this is a good idea, but should be added later.
> * Introduce cbindgen to avoid desync errors between Rust and C.
> * LIke other points above; This should be added later.
Agreed, these all make sense to me.
> * What should we call the crate that will be the new home for .c and
> .h files in the root of Git?
We could call this something like "libgit-ffi", but I don't care too
much.
Thanks!
Patrick |
|
On the Git mailing list, Patrick Steinhardt wrote (reply to this): On Wed, Sep 17, 2025 at 03:48:04PM -0700, Junio C Hamano wrote:
> Elijah Newren <[email protected]> writes:
>
> > So, how to move forward?
> >
> > A) Modify Patrick's series to just take patch 7 of his v5. Patrick
> > did say that the roadmap was "the more important discussion compared
> > to the technical discussion", and merging that patch would achieve his
> > goal of getting an initial roadmap. Then Ezekiel could grab other
> > pieces from Patrick's series (e.g. the help and varint stuff) and
> > incorporate it into an "introduce rust" series.[*]
> >
> > B) Merge Patrick's series and tell Ezekiel to rebase, while noting to
> > Ezekiel that the roadmap is the important bit from Patrick's series[*]
> > and he can suggest changes to any of the other bits.
> >
> > C) Create a consolidated "introduce Rust" series with bits of both --
> > what I think Ezekiel was trying to do with this series.
>
> Ah, I didn't even realize C was what this series was trying to do.
>
> I do not have particular preference between A and B, but I thought A
> was closer to what was being done with this series, and as long as
> Ezekiel and Patrick can join forces that way, it would be perfect.
I personally think either (A) or (B) would be good choices. I would
slightly lean towards (B) just so that we have something that we can
already play around with while building the next steps.
By the way: I'm also happy to change attribution of some of the patches
in my patch series to mention Ezekiel as author. I don't care much who
is listed for the initial patches that introduce Rust, but would retain
my own authorship for the "varint" and "BreakingChanges" commits.
Patrick |
|
On the Git mailing list, Ezekiel Newren wrote (reply to this): On Mon, Sep 22, 2025 at 7:01 AM Patrick Steinhardt <[email protected]> wrote:
> I personally think either (A) or (B) would be good choices. I would
> slightly lean towards (B) just so that we have something that we can
> already play around with while building the next steps.
I'm fine with B if you fix the wording in your Breaking Changes about
Rust being introduced in version 2.52. Rust was introduced to Git in
2.49.
Elijah points this out in 1 and 2:
[1] https://lore.kernel.org/git/CABPp-BFXRbaHk9U3BX+d12bZ+ryGOp+btR0ODMw+HtD7xd+MBQ@mail.gmail.com/
[2] https://lore.kernel.org/git/CABPp-BEiK49f_UB5UPe3qM9O7vQGGFJ8Nshw1f6W_6Lw7HRL6Q@mail.gmail.com/
> By the way: I'm also happy to change attribution of some of the patches
> in my patch series to mention Ezekiel as author. I don't care much who
> is listed for the initial patches that introduce Rust, but would retain
> my own authorship for the "varint" and "BreakingChanges" commits.
My only other concern is with varint. You use usize on the Rust side
and then uint64_t on the C side, but I'm ok with fixing that later as
it only breaks 'linux32 (i386/ubuntu:focal)' in the github workflows. |
|
On the Git mailing list, Ezekiel Newren wrote (reply to this): On Mon, Sep 22, 2025 at 7:01 AM Patrick Steinhardt <[email protected]> wrote:
> I don't think there'd have to be major changes to the current version
> of my patch series. The idea of that patch series is very much getting
> buy-in regarding our roadmap and focus less on the actual build infra.
> So the changes introduced are mostly an MVC, and I very much think that
> we'll have to iterate quite a bit on it, but that's intended.
So long as you're flexible with the build infra details then I'm ok
with rebasing on your work.
What does MVC stand for?
> I think that a lot of the steps you outline below are logical next steps
> to get there. If I can change anything to make these next steps easier
> for you I'm happy to do so. But I also don't think it's too bad if we
> have to change the current infra quite significantly to get there.
I think we can discuss those code and infra changes in a later patch series.
> > I wanted feedback on:
> > * Cleaning up Rust type name collisions
> > * People don't like it, so I'll drop that
>
> I don't have a strong opinion on this. If it creates issues I personally
> don't mind fixing it.
Junio doesn't like it, so I'm not going to do it.
> > * Have Makefile produce libgit.a correctly.
> > * I think this is a good idea, but it doesn't belong in this patch series.
> > * Adding Rust unit test which required fixing Make and adding
> > build-helper to reduce boilerplate code
> > * I think this is a good idea, but should be added later.
> > * Introduce cbindgen to avoid desync errors between Rust and C.
> > * LIke other points above; This should be added later.
>
> Agreed, these all make sense to me.
I think the changes to Makefile to build ligbit.a correctly can be an
orthogonal patch series. The rest can come later.
> > * What should we call the crate that will be the new home for .c and
> > .h files in the root of Git?
>
> We could call this something like "libgit-ffi", but I don't care too
> much.
I prefer Eric Sunshine's suggestion of gitcore, no hyphen. |
|
On the Git mailing list, Patrick Steinhardt wrote (reply to this): On Mon, Sep 22, 2025 at 09:31:10AM -0600, Ezekiel Newren wrote:
> On Mon, Sep 22, 2025 at 7:01 AM Patrick Steinhardt <[email protected]> wrote:
> > I don't think there'd have to be major changes to the current version
> > of my patch series. The idea of that patch series is very much getting
> > buy-in regarding our roadmap and focus less on the actual build infra.
> > So the changes introduced are mostly an MVC, and I very much think that
> > we'll have to iterate quite a bit on it, but that's intended.
>
> So long as you're flexible with the build infra details then I'm ok
> with rebasing on your work.
>
> What does MVC stand for?
Minimum viable... candidate? I guess I rather meant "product", so MVP
would be the better term.
[snip]
> > > * What should we call the crate that will be the new home for .c and
> > > .h files in the root of Git?
> >
> > We could call this something like "libgit-ffi", but I don't care too
> > much.
>
> I prefer Eric Sunshine's suggestion of gitcore, no hyphen.
Sounds reasonable to me.
Patrick |
|
On the Git mailing list, Patrick Steinhardt wrote (reply to this): On Mon, Sep 22, 2025 at 09:18:14AM -0600, Ezekiel Newren wrote:
> On Mon, Sep 22, 2025 at 7:01 AM Patrick Steinhardt <[email protected]> wrote:
> > I personally think either (A) or (B) would be good choices. I would
> > slightly lean towards (B) just so that we have something that we can
> > already play around with while building the next steps.
>
> I'm fine with B if you fix the wording in your Breaking Changes about
> Rust being introduced in version 2.52. Rust was introduced to Git in
> 2.49.
>
> Elijah points this out in 1 and 2:
> [1] https://lore.kernel.org/git/CABPp-BFXRbaHk9U3BX+d12bZ+ryGOp+btR0ODMw+HtD7xd+MBQ@mail.gmail.com/
> [2] https://lore.kernel.org/git/CABPp-BEiK49f_UB5UPe3qM9O7vQGGFJ8Nshw1f6W_6Lw7HRL6Q@mail.gmail.com/
Will adjust.
> > By the way: I'm also happy to change attribution of some of the patches
> > in my patch series to mention Ezekiel as author. I don't care much who
> > is listed for the initial patches that introduce Rust, but would retain
> > my own authorship for the "varint" and "BreakingChanges" commits.
>
> My only other concern is with varint. You use usize on the Rust side
> and then uint64_t on the C side, but I'm ok with fixing that later as
> it only breaks 'linux32 (i386/ubuntu:focal)' in the github workflows.
Oh, this is actually an oversight, good catch! I refactored "varint.c"
to use `uint64_t`, but then forgot to adjust the Rust side in the same
spirit. Will fix.
I suggested in [1] that I can change authorship of the patches that
introduce the initial infrastructure into Meson and our Makefile (so I
guess patches 1 and 3) to instead list you as author and myself as
Co-authored-by. Is that something you want? Given that you have
kickstarted the whole effort around introducing Rust again I wouldn't
mind that at all.
In any case, I'll send a new version of the series tomorrow.
Thanks!
Patrick
[1]: <[email protected]> |
|
On the Git mailing list, Ezekiel Newren wrote (reply to this): On Mon, Sep 22, 2025 at 10:16 AM Patrick Steinhardt <[email protected]> wrote:
> On Mon, Sep 22, 2025 at 09:18:14AM -0600, Ezekiel Newren wrote:
> > On Mon, Sep 22, 2025 at 7:01 AM Patrick Steinhardt <[email protected]> wrote:
> > > I personally think either (A) or (B) would be good choices. I would
> > > slightly lean towards (B) just so that we have something that we can
> > > already play around with while building the next steps.
> >
> > I'm fine with B if you fix the wording in your Breaking Changes about
> > Rust being introduced in version 2.52. Rust was introduced to Git in
> > 2.49.
> >
> > Elijah points this out in 1 and 2:
> > [1] https://lore.kernel.org/git/CABPp-BFXRbaHk9U3BX+d12bZ+ryGOp+btR0ODMw+HtD7xd+MBQ@mail.gmail.com/
> > [2] https://lore.kernel.org/git/CABPp-BEiK49f_UB5UPe3qM9O7vQGGFJ8Nshw1f6W_6Lw7HRL6Q@mail.gmail.com/
>
> Will adjust.
Thank you.
> > > By the way: I'm also happy to change attribution of some of the patches
> > > in my patch series to mention Ezekiel as author. I don't care much who
> > > is listed for the initial patches that introduce Rust, but would retain
> > > my own authorship for the "varint" and "BreakingChanges" commits.
> >
> > My only other concern is with varint. You use usize on the Rust side
> > and then uint64_t on the C side, but I'm ok with fixing that later as
> > it only breaks 'linux32 (i386/ubuntu:focal)' in the github workflows.
>
> Oh, this is actually an oversight, good catch! I refactored "varint.c"
> to use `uint64_t`, but then forgot to adjust the Rust side in the same
> spirit. Will fix.
You also missed updating varint.h.
> I suggested in [1] that I can change authorship of the patches that
> introduce the initial infrastructure into Meson and our Makefile (so I
> guess patches 1 and 3) to instead list you as author and myself as
> Co-authored-by. Is that something you want? Given that you have
> kickstarted the whole effort around introducing Rust again I wouldn't
> mind that at all.
It doesn't make sense to me to list myself as the author of any of
your commits, but I would like my name referenced in your commit
messages.
Thanks.
Ezekiel. |
|
On the Git mailing list, Junio C Hamano wrote (reply to this): Ezekiel Newren <[email protected]> writes:
>> > I wanted feedback on:
>> > * Cleaning up Rust type name collisions
>> > * People don't like it, so I'll drop that
>>
>> I don't have a strong opinion on this. If it creates issues I personally
>> don't mind fixing it.
>
> Junio doesn't like it, so I'm not going to do it.
It was not "I do not line u16 as a typename when a perfectly well
established uint16_t is available", though.
It was more about asking to explain the reason behind insisting to
use u(8|16|32|64) types in C code. Perhaps there is a compelling
reason to do so that I was missing.
I know that the kernel has used these types for a long time, but
that way predates their more recent flirt with Rust. If your answer
was "the kernel uses them", then I'd want that answer to cover a few
additional questions, like
- Have they benefitted from their use of u(8|16|32|64) when they
started working with Rust and if so how?
- Would we be expected to reap the same benefit if we used these
types?
for example.
Thanks. |
|
On the Git mailing list, Ezekiel Newren wrote (reply to this): On Mon, Sep 22, 2025 at 10:47 AM Junio C Hamano <[email protected]> wrote:
> Ezekiel Newren <[email protected]> writes:
>
> >> > I wanted feedback on:
> >> > * Cleaning up Rust type name collisions
> >> > * People don't like it, so I'll drop that
> >>
> >> I don't have a strong opinion on this. If it creates issues I personally
> >> don't mind fixing it.
> >
> > Junio doesn't like it, so I'm not going to do it.
>
> It was not "I do not like u16 as a typename when a perfectly well
> established uint16_t is available", though.
>
> It was more about asking to explain the reason behind insisting to
> use u(8|16|32|64) types in C code. Perhaps there is a compelling
> reason to do so that I was missing.
I listed 5 reasons in my commit message[1], and I just thought of
another one now.
* Pseudo reserved keywords: 'new' is not a reserved keyword in C,
but Git treats it as such. Since Rust will likely be added to Git, the
Rust primitive types should also be treated as reserved keywords.
Cbindgen parse's Rust and generates C header files; If a field in a
struct uses u16 as the name then Rust won't compile, and cbindgen
can't create the C header file. Using [ui](8|16|32|64|size) as the
type in C also spreads awareness that those are reserved keywords and
should not be used as variable names.
If these 6 reasons are not enough to explain why we should be using
the Rust primitive type names in C, then please explain how it is
insufficient.
> I know that the kernel has used these types for a long time, but
> that way predates their more recent flirt with Rust. If your answer
> was "the kernel uses them", then I'd want that answer to cover a few
> additional questions, like
>
> - Have they benefited from their use of u(8|16|32|64) when they
> started working with Rust and if so how?
I did not know this.
[1] https://lore.kernel.org/git/2a7d5b05c18d4a96f1905b7043d47c62d367cd2a.1757274320.git.gitgitgadget@gmail.com/ |
|
On the Git mailing list, Ezekiel Newren wrote (reply to this): On Mon, Sep 22, 2025 at 11:23 AM Ezekiel Newren <[email protected]> wrote:
> Cbindgen parse's Rust and generates C header files; If a field in a
> struct uses u16 as the name then Rust won't compile, and cbindgen
> can't create the C header file.
I just tried this in Rust and it turns out you actually can use u16 as
a struct field name. My bad. I think that's a bad idea and should be
discouraged though. |
|
On the Git mailing list, Junio C Hamano wrote (reply to this): Ezekiel Newren <[email protected]> writes:
> If a field in a
> struct uses u16 as the name then Rust won't compile, and cbindgen
> can't create the C header file.
Do you mean that cbindgen fails to help us make a C code we have
that says "struct { uint16_t u16; }" to work with Rust?
Declaring "typedef uint16_t u16" would not help such a case at all,
as it would not make "struct { u16 u16; }" an invalid C, and would
not force us to avoid such names that cbindgen may have problems
with.
So regardless of what to do with type names, we would need to adjust
some variable names to avoid clashes with Rust, which I am fine with.
> Using [ui](8|16|32|64|size) as the type in C also spreads
> awareness that those are reserved keywords and should not be used
> as variable names.
We certainly need to train our developers to avoid problematic names
like "u16" just like we do so for "new".
I am skeptical that using u16 as a type would have a good chance to
contribute to that effort (otherwise we would have added "new" as a
type to solve this issue already), but I am willing to be talked
into trying, with a few conditions to prevent unnecessary churning,
i.e. We do "typedef uint16_t u16" and friends, and new and old code
that are written to directly interact with Rust written code would
be better written with u16 and friends, so the same thing is called
similarly across the wall, but we want to avoid replacing uint16_t
with u16 blindly.
|
|
On the Git mailing list, Junio C Hamano wrote (reply to this): Ezekiel Newren <[email protected]> writes:
> On Mon, Sep 22, 2025 at 11:23 AM Ezekiel Newren <[email protected]> wrote:
>> Cbindgen parse's Rust and generates C header files; If a field in a
>> struct uses u16 as the name then Rust won't compile, and cbindgen
>> can't create the C header file.
>
> I just tried this in Rust and it turns out you actually can use u16 as
> a struct field name. My bad. I think that's a bad idea and should be
> discouraged though.
That's a great news.
It means we do not have to worry about existing variables and
structure member names at all while working with cbindgen.
|
|
On the Git mailing list, Ezekiel Newren wrote (reply to this): On Mon, Sep 22, 2025 at 12:17 PM Junio C Hamano <[email protected]> wrote:
> > I just tried this in Rust and it turns out you actually can use u16 as
> > a struct field name. My bad. I think that's a bad idea and should be
> > discouraged though.
>
> That's a great news.
>
> It means we do not have to worry about existing variables and
> structure member names at all while working with cbindgen.
You've convinced me that we shouldn't use Rust type names in C. I've
already refactored my code to use [ui]int(8|16|32|64)_t in part 2 of
my xdiff cleanup. |
|
On the Git mailing list, Junio C Hamano wrote (reply to this): Ezekiel Newren <[email protected]> writes:
> already refactored my code to use [ui]int(8|16|32|64)_t in part 2 of
> my xdiff cleanup.
Noted. I think [u]int(8|16|32|64)_t would be familiar to both C
writers and Rust folks who need to peek into C for working with it.
Thanks. |
|
This patch series was integrated into seen via 2d7d562. |
|
There was a status update in the "Cooking" section about the branch Expecting a reroll. source: <[email protected]> |
|
On the Git mailing list, Patrick Steinhardt wrote (reply to this): On Mon, Sep 22, 2025 at 10:27:32AM -0600, Ezekiel Newren wrote:
> On Mon, Sep 22, 2025 at 10:16 AM Patrick Steinhardt <[email protected]> wrote:
> > On Mon, Sep 22, 2025 at 09:18:14AM -0600, Ezekiel Newren wrote:
> > > > By the way: I'm also happy to change attribution of some of the patches
> > > > in my patch series to mention Ezekiel as author. I don't care much who
> > > > is listed for the initial patches that introduce Rust, but would retain
> > > > my own authorship for the "varint" and "BreakingChanges" commits.
> > >
> > > My only other concern is with varint. You use usize on the Rust side
> > > and then uint64_t on the C side, but I'm ok with fixing that later as
> > > it only breaks 'linux32 (i386/ubuntu:focal)' in the github workflows.
> >
> > Oh, this is actually an oversight, good catch! I refactored "varint.c"
> > to use `uint64_t`, but then forgot to adjust the Rust side in the same
> > spirit. Will fix.
>
> You also missed updating varint.h.
Hm, am I missing anything? It does use `uint64_t`, and if it didn't it
would cause a compiler error due to mismatching declarations.
> > I suggested in [1] that I can change authorship of the patches that
> > introduce the initial infrastructure into Meson and our Makefile (so I
> > guess patches 1 and 3) to instead list you as author and myself as
> > Co-authored-by. Is that something you want? Given that you have
> > kickstarted the whole effort around introducing Rust again I wouldn't
> > mind that at all.
>
> It doesn't make sense to me to list myself as the author of any of
> your commits, but I would like my name referenced in your commit
> messages.
Okay, will do. Is it sufficient if I say something "Based-on-patch-by"
or "Inspired-by"? Don't really have much of a better idea for how to
include it, but please let me know in case you have any preference.
Patrick |
|
On the Git mailing list, Ezekiel Newren wrote (reply to this): On Mon, Sep 22, 2025 at 11:11 PM Patrick Steinhardt <[email protected]> wrote:
>
> On Mon, Sep 22, 2025 at 10:27:32AM -0600, Ezekiel Newren wrote:
> > On Mon, Sep 22, 2025 at 10:16 AM Patrick Steinhardt <[email protected]> wrote:
> > > On Mon, Sep 22, 2025 at 09:18:14AM -0600, Ezekiel Newren wrote:
> > > > > By the way: I'm also happy to change attribution of some of the patches
> > > > > in my patch series to mention Ezekiel as author. I don't care much who
> > > > > is listed for the initial patches that introduce Rust, but would retain
> > > > > my own authorship for the "varint" and "BreakingChanges" commits.
> > > >
> > > > My only other concern is with varint. You use usize on the Rust side
> > > > and then uint64_t on the C side, but I'm ok with fixing that later as
> > > > it only breaks 'linux32 (i386/ubuntu:focal)' in the github workflows.
> > >
> > > Oh, this is actually an oversight, good catch! I refactored "varint.c"
> > > to use `uint64_t`, but then forgot to adjust the Rust side in the same
> > > spirit. Will fix.
> >
> > You also missed updating varint.h.
Oh, I think when I was editing your patches locally varint.h got out of sync.
> Hm, am I missing anything? It does use `uint64_t`, and if it didn't it
> would cause a compiler error due to mismatching declarations.
unsigned char should be replaced with uint8_t. I don't know the exact
location that causes linux32 (i386/ubuntu:focal) to fail, I think it
had to do with specifying uint64_t on the C side, and usize on the
Rust side. usize is 32-bits on i386/ubuntu:focal.
> > > I suggested in [1] that I can change authorship of the patches that
> > > introduce the initial infrastructure into Meson and our Makefile (so I
> > > guess patches 1 and 3) to instead list you as author and myself as
> > > Co-authored-by. Is that something you want? Given that you have
> > > kickstarted the whole effort around introducing Rust again I wouldn't
> > > mind that at all.
> >
> > It doesn't make sense to me to list myself as the author of any of
> > your commits, but I would like my name referenced in your commit
> > messages.
>
> Okay, will do. Is it sufficient if I say something "Based-on-patch-by"
> or "Inspired-by"? Don't really have much of a better idea for how to
> include it, but please let me know in case you have any preference.
Inspired-by would be my preference. |
|
On the Git mailing list, Ezekiel Newren wrote (reply to this): On Tue, Sep 16, 2025 at 7:16 PM Ezekiel Newren via GitGitGadget
<[email protected]> wrote:
DROP PATCH SERIES: I am dropping this patch series in favor of
Patrick's. Everything I want to add can be rebased on top of his work. |
|
On the Git mailing list, Junio C Hamano wrote (reply to this): Ezekiel Newren <[email protected]> writes:
> On Tue, Sep 16, 2025 at 7:16 PM Ezekiel Newren via GitGitGadget
> <[email protected]> wrote:
> DROP PATCH SERIES: I am dropping this patch series in favor of
> Patrick's. Everything I want to add can be rebased on top of his work.
OK. Thanks for expressing intentions clearly. Will adjust my end
by ejecting them from 'seen'.
|
webstech
added a commit
to gitgitgadget/git
that referenced
this pull request
Nov 5, 2025
…it#2043) Bumps [@stylistic/eslint-plugin](https://github.com/eslint-stylistic/eslint-stylistic/tree/HEAD/packages/eslint-plugin) from 5.4.0 to 5.5.0. <details> <summary>Release notes</summary> <p><em>Sourced from <a href="https://github.com/eslint-stylistic/eslint-stylistic/releases"><code>@stylistic/eslint-plugin</code>'s releases</a>.</em></p> <blockquote> <h2>v5.5.0</h2> <h2><a href="https://github.com/eslint-stylistic/eslint-stylistic/compare/v5.4.0...v5.5.0">5.5.0</a> (2025-10-18)</h2> <h3>Features</h3> <ul> <li><strong>comma-dangle:</strong> support <code>TSFunctionType</code> and <code>TSDeclareFunction</code> (<a href="https://redirect.github.com/eslint-stylistic/eslint-stylistic/issues/1015">#1015</a>) (<a href="https://github.com/eslint-stylistic/eslint-stylistic/commit/cd3776f8a9c66fdc8ac0ee523734c8b1e30d0859">cd3776f</a>)</li> <li><strong>comma-dangle:</strong> support <code>TSTypeParameterInstantiation</code> (<a href="https://redirect.github.com/eslint-stylistic/eslint-stylistic/issues/1016">#1016</a>) (<a href="https://github.com/eslint-stylistic/eslint-stylistic/commit/ba930e7269351ea1bfc423dd8036e9658438f25c">ba930e7</a>)</li> <li><strong>indent:</strong> deprecate <code>offsetTernaryExpressionsOffsetCallExpressions</code> via <code>offsetTernaryExpressions.CallExpression</code> (<a href="https://redirect.github.com/eslint-stylistic/eslint-stylistic/issues/997">#997</a>) (<a href="https://github.com/eslint-stylistic/eslint-stylistic/commit/f2837b1d9a5d74bef64f660846f5ac78684aaed0">f2837b1</a>)</li> <li><strong>indent:</strong> introduce <code>NewExpression</code> and <code>AwaitExpression</code> in <code>offsetTernaryExpressions</code> (<a href="https://redirect.github.com/eslint-stylistic/eslint-stylistic/issues/996">#996</a>) (<a href="https://github.com/eslint-stylistic/eslint-stylistic/commit/2b5a39fc985a2c9263736f43ae362a86c4426281">2b5a39f</a>)</li> <li>new rule list-style (<a href="https://redirect.github.com/eslint-stylistic/eslint-stylistic/issues/895">#895</a>) (<a href="https://github.com/eslint-stylistic/eslint-stylistic/commit/a9ec0de85d61c9e7452dac1942fff46684c379ed">a9ec0de</a>)</li> <li><strong>object-curly-spacing:</strong> add <code>emptyObjects</code> option to control spacing in empty objects (<a href="https://redirect.github.com/eslint-stylistic/eslint-stylistic/issues/1002">#1002</a>) (<a href="https://github.com/eslint-stylistic/eslint-stylistic/commit/85ef5bcf052bdd4646abed86130b4fc8d7d948fb">85ef5bc</a>)</li> <li><strong>padding-line-between-statements:</strong> narrow report range to reduce noise (<a href="https://redirect.github.com/eslint-stylistic/eslint-stylistic/issues/1017">#1017</a>) (<a href="https://github.com/eslint-stylistic/eslint-stylistic/commit/b3a3acffbfa88d6cc0c0072e9e46129cfeeb9a98">b3a3acf</a>)</li> <li>update deps (<a href="https://redirect.github.com/eslint-stylistic/eslint-stylistic/issues/1011">#1011</a>) (<a href="https://github.com/eslint-stylistic/eslint-stylistic/commit/9d5085c0309e1b6868838c5a992fce2b703e023d">9d5085c</a>)</li> </ul> <h3>Bug Fixes</h3> <ul> <li><strong>indent:</strong> correctly indent <code>NewExpression</code> in <code>ConditionalExpression</code> (<a href="https://redirect.github.com/eslint-stylistic/eslint-stylistic/issues/994">#994</a>) (<a href="https://github.com/eslint-stylistic/eslint-stylistic/commit/01cec33474304bcb0365ab7d9ca8f4b504529e82">01cec33</a>)</li> <li>replace <code>context.getSourceCode()</code> with <code>context.sourceCode</code> (<a href="https://redirect.github.com/eslint-stylistic/eslint-stylistic/issues/1004">#1004</a>) (<a href="https://github.com/eslint-stylistic/eslint-stylistic/commit/f3faa54295621c5eddbbefbf4630974770f60a69">f3faa54</a>)</li> </ul> <h3>Documentation</h3> <ul> <li><strong>contribute:</strong> add guide to run the document site (<a href="https://redirect.github.com/eslint-stylistic/eslint-stylistic/issues/1001">#1001</a>) (<a href="https://github.com/eslint-stylistic/eslint-stylistic/commit/37a64c5b9c1edd3c184d9d112e1539d06ddc08dc">37a64c5</a>)</li> <li><strong>object-curly-spacing:</strong> introduce <code>overrides</code> (<a href="https://redirect.github.com/eslint-stylistic/eslint-stylistic/issues/998">#998</a>) (<a href="https://github.com/eslint-stylistic/eslint-stylistic/commit/6452b523a48f4f21a3c1d37ca70c6246ce9fa897">6452b52</a>)</li> </ul> <h3>Chores</h3> <ul> <li><strong>no-extra-parens:</strong> remove useless logic (<a href="https://redirect.github.com/eslint-stylistic/eslint-stylistic/issues/1013">#1013</a>) (<a href="https://github.com/eslint-stylistic/eslint-stylistic/commit/8e34765bed7ed363386d8d38e27a34b605c4fed6">8e34765</a>)</li> <li>remove <code>TestCaseError#type</code> (<a href="https://redirect.github.com/eslint-stylistic/eslint-stylistic/issues/1005">#1005</a>) (<a href="https://github.com/eslint-stylistic/eslint-stylistic/commit/83d00c5735a3ac5a69fc053c3714cd8ca14ab5f4">83d00c5</a>)</li> <li>replace <code>Object.prototype.hasOwnProperty.call</code> with <code>Object.hasOwn</code> (<a href="https://redirect.github.com/eslint-stylistic/eslint-stylistic/issues/992">#992</a>) (<a href="https://github.com/eslint-stylistic/eslint-stylistic/commit/97193e14d60d9e46b9c9a5cde3be5f2f35e9c251">97193e1</a>)</li> </ul> </blockquote> </details> <details> <summary>Changelog</summary> <p><em>Sourced from <a href="https://github.com/eslint-stylistic/eslint-stylistic/blob/main/CHANGELOG.md"><code>@stylistic/eslint-plugin</code>'s changelog</a>.</em></p> <blockquote> <h2><a href="https://github.com/eslint-stylistic/eslint-stylistic/compare/v5.4.0...v5.5.0">5.5.0</a> (2025-10-18)</h2> <h3>Features</h3> <ul> <li><strong>comma-dangle:</strong> support <code>TSFunctionType</code> and <code>TSDeclareFunction</code> (<a href="https://redirect.github.com/eslint-stylistic/eslint-stylistic/issues/1015">#1015</a>) (<a href="https://github.com/eslint-stylistic/eslint-stylistic/commit/cd3776f8a9c66fdc8ac0ee523734c8b1e30d0859">cd3776f</a>)</li> <li><strong>comma-dangle:</strong> support <code>TSTypeParameterInstantiation</code> (<a href="https://redirect.github.com/eslint-stylistic/eslint-stylistic/issues/1016">#1016</a>) (<a href="https://github.com/eslint-stylistic/eslint-stylistic/commit/ba930e7269351ea1bfc423dd8036e9658438f25c">ba930e7</a>)</li> <li><strong>indent:</strong> deprecate <code>offsetTernaryExpressionsOffsetCallExpressions</code> via <code>offsetTernaryExpressions.CallExpression</code> (<a href="https://redirect.github.com/eslint-stylistic/eslint-stylistic/issues/997">#997</a>) (<a href="https://github.com/eslint-stylistic/eslint-stylistic/commit/f2837b1d9a5d74bef64f660846f5ac78684aaed0">f2837b1</a>)</li> <li><strong>indent:</strong> introduce <code>NewExpression</code> and <code>AwaitExpression</code> in <code>offsetTernaryExpressions</code> (<a href="https://redirect.github.com/eslint-stylistic/eslint-stylistic/issues/996">#996</a>) (<a href="https://github.com/eslint-stylistic/eslint-stylistic/commit/2b5a39fc985a2c9263736f43ae362a86c4426281">2b5a39f</a>)</li> <li>new rule list-style (<a href="https://redirect.github.com/eslint-stylistic/eslint-stylistic/issues/895">#895</a>) (<a href="https://github.com/eslint-stylistic/eslint-stylistic/commit/a9ec0de85d61c9e7452dac1942fff46684c379ed">a9ec0de</a>)</li> <li><strong>object-curly-spacing:</strong> add <code>emptyObjects</code> option to control spacing in empty objects (<a href="https://redirect.github.com/eslint-stylistic/eslint-stylistic/issues/1002">#1002</a>) (<a href="https://github.com/eslint-stylistic/eslint-stylistic/commit/85ef5bcf052bdd4646abed86130b4fc8d7d948fb">85ef5bc</a>)</li> <li><strong>padding-line-between-statements:</strong> narrow report range to reduce noise (<a href="https://redirect.github.com/eslint-stylistic/eslint-stylistic/issues/1017">#1017</a>) (<a href="https://github.com/eslint-stylistic/eslint-stylistic/commit/b3a3acffbfa88d6cc0c0072e9e46129cfeeb9a98">b3a3acf</a>)</li> <li>update deps (<a href="https://redirect.github.com/eslint-stylistic/eslint-stylistic/issues/1011">#1011</a>) (<a href="https://github.com/eslint-stylistic/eslint-stylistic/commit/9d5085c0309e1b6868838c5a992fce2b703e023d">9d5085c</a>)</li> </ul> <h3>Bug Fixes</h3> <ul> <li><strong>indent:</strong> correctly indent <code>NewExpression</code> in <code>ConditionalExpression</code> (<a href="https://redirect.github.com/eslint-stylistic/eslint-stylistic/issues/994">#994</a>) (<a href="https://github.com/eslint-stylistic/eslint-stylistic/commit/01cec33474304bcb0365ab7d9ca8f4b504529e82">01cec33</a>)</li> <li>replace <code>context.getSourceCode()</code> with <code>context.sourceCode</code> (<a href="https://redirect.github.com/eslint-stylistic/eslint-stylistic/issues/1004">#1004</a>) (<a href="https://github.com/eslint-stylistic/eslint-stylistic/commit/f3faa54295621c5eddbbefbf4630974770f60a69">f3faa54</a>)</li> </ul> <h3>Documentation</h3> <ul> <li><strong>contribute:</strong> add guide to run the document site (<a href="https://redirect.github.com/eslint-stylistic/eslint-stylistic/issues/1001">#1001</a>) (<a href="https://github.com/eslint-stylistic/eslint-stylistic/commit/37a64c5b9c1edd3c184d9d112e1539d06ddc08dc">37a64c5</a>)</li> <li><strong>object-curly-spacing:</strong> introduce <code>overrides</code> (<a href="https://redirect.github.com/eslint-stylistic/eslint-stylistic/issues/998">#998</a>) (<a href="https://github.com/eslint-stylistic/eslint-stylistic/commit/6452b523a48f4f21a3c1d37ca70c6246ce9fa897">6452b52</a>)</li> </ul> <h3>Chores</h3> <ul> <li><strong>no-extra-parens:</strong> remove useless logic (<a href="https://redirect.github.com/eslint-stylistic/eslint-stylistic/issues/1013">#1013</a>) (<a href="https://github.com/eslint-stylistic/eslint-stylistic/commit/8e34765bed7ed363386d8d38e27a34b605c4fed6">8e34765</a>)</li> <li>remove <code>TestCaseError#type</code> (<a href="https://redirect.github.com/eslint-stylistic/eslint-stylistic/issues/1005">#1005</a>) (<a href="https://github.com/eslint-stylistic/eslint-stylistic/commit/83d00c5735a3ac5a69fc053c3714cd8ca14ab5f4">83d00c5</a>)</li> <li>replace <code>Object.prototype.hasOwnProperty.call</code> with <code>Object.hasOwn</code> (<a href="https://redirect.github.com/eslint-stylistic/eslint-stylistic/issues/992">#992</a>) (<a href="https://github.com/eslint-stylistic/eslint-stylistic/commit/97193e14d60d9e46b9c9a5cde3be5f2f35e9c251">97193e1</a>)</li> </ul> </blockquote> </details> <details> <summary>Commits</summary> <ul> <li><a href="https://github.com/eslint-stylistic/eslint-stylistic/commit/ad0c6a9432b23058a46d7f68840c2755c1284263"><code>ad0c6a9</code></a> chore: release v5.5.0 (main) (<a href="https://github.com/eslint-stylistic/eslint-stylistic/tree/HEAD/packages/eslint-plugin/issues/995">#995</a>)</li> <li><a href="https://github.com/eslint-stylistic/eslint-stylistic/commit/ba930e7269351ea1bfc423dd8036e9658438f25c"><code>ba930e7</code></a> feat(comma-dangle): support <code>TSTypeParameterInstantiation</code> (<a href="https://github.com/eslint-stylistic/eslint-stylistic/tree/HEAD/packages/eslint-plugin/issues/1016">#1016</a>)</li> <li><a href="https://github.com/eslint-stylistic/eslint-stylistic/commit/b3a3acffbfa88d6cc0c0072e9e46129cfeeb9a98"><code>b3a3acf</code></a> feat(padding-line-between-statements): narrow report range to reduce noise (#...</li> <li><a href="https://github.com/eslint-stylistic/eslint-stylistic/commit/cd3776f8a9c66fdc8ac0ee523734c8b1e30d0859"><code>cd3776f</code></a> feat(comma-dangle): support <code>TSFunctionType</code> and <code>TSDeclareFunction</code> (<a href="https://github.com/eslint-stylistic/eslint-stylistic/tree/HEAD/packages/eslint-plugin/issues/1015">#1015</a>)</li> <li><a href="https://github.com/eslint-stylistic/eslint-stylistic/commit/8e34765bed7ed363386d8d38e27a34b605c4fed6"><code>8e34765</code></a> refactor(no-extra-parens): remove useless logic (<a href="https://github.com/eslint-stylistic/eslint-stylistic/tree/HEAD/packages/eslint-plugin/issues/1013">#1013</a>)</li> <li><a href="https://github.com/eslint-stylistic/eslint-stylistic/commit/9d5085c0309e1b6868838c5a992fce2b703e023d"><code>9d5085c</code></a> feat: update deps (<a href="https://github.com/eslint-stylistic/eslint-stylistic/tree/HEAD/packages/eslint-plugin/issues/1011">#1011</a>)</li> <li><a href="https://github.com/eslint-stylistic/eslint-stylistic/commit/f2837b1d9a5d74bef64f660846f5ac78684aaed0"><code>f2837b1</code></a> feat(indent): deprecate <code>offsetTernaryExpressionsOffsetCallExpressions</code> via `...</li> <li><a href="https://github.com/eslint-stylistic/eslint-stylistic/commit/85ef5bcf052bdd4646abed86130b4fc8d7d948fb"><code>85ef5bc</code></a> feat(object-curly-spacing): add <code>emptyObjects</code> option to control spacing in e...</li> <li><a href="https://github.com/eslint-stylistic/eslint-stylistic/commit/a9ec0de85d61c9e7452dac1942fff46684c379ed"><code>a9ec0de</code></a> feat: new rule list-style (<a href="https://github.com/eslint-stylistic/eslint-stylistic/tree/HEAD/packages/eslint-plugin/issues/895">#895</a>)</li> <li><a href="https://github.com/eslint-stylistic/eslint-stylistic/commit/2b5a39fc985a2c9263736f43ae362a86c4426281"><code>2b5a39f</code></a> feat(indent): introduce <code>NewExpression</code> and <code>AwaitExpression</code> in `offsetTerna...</li> <li>Additional commits viewable in <a href="https://github.com/eslint-stylistic/eslint-stylistic/commits/v5.5.0/packages/eslint-plugin">compare view</a></li> </ul> </details> <br /> [](https://docs.github.com/en/github/managing-security-vulnerabilities/about-dependabot-security-updates#about-compatibility-scores) Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting `@dependabot rebase`. [//]: # (dependabot-automerge-start) [//]: # (dependabot-automerge-end) --- <details> <summary>Dependabot commands and options</summary> <br /> You can trigger Dependabot actions by commenting on this PR: - `@dependabot rebase` will rebase this PR - `@dependabot recreate` will recreate this PR, overwriting any edits that have been made to it - `@dependabot merge` will merge this PR after your CI passes on it - `@dependabot squash and merge` will squash and merge this PR after your CI passes on it - `@dependabot cancel merge` will cancel a previously requested merge and block automerging - `@dependabot reopen` will reopen this PR if it is closed - `@dependabot close` will close this PR and stop Dependabot recreating it. You can achieve the same result by closing it manually - `@dependabot show <dependency name> ignore conditions` will show all of the ignore conditions of the specified dependency - `@dependabot ignore this major version` will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this minor version` will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this dependency` will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself) </details>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is a continuation of https://lore.kernel.org/git/[email protected]/, 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:
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.
Changes in v1:
Changes since the last RFC patch series (range-diff added below):
High level overview:
cc: Patrick Steinhardt [email protected]
cc: Eric Sunshine [email protected]
cc: Elijah Newren [email protected]
cc: "D. Ben Knoble" [email protected]
cc: Eric Sunshine [email protected]
cc: Collin Funk [email protected]
cc: Ramsay Jones [email protected]