RFC: Accelerate xdiff and begin its rustification

rust/Cargo.lock new file +4 -0

new file mode 100644
index 000000000000..fb1eac690b39
@@ -0,0 +1,14 @@
+# This file is automatically @generated by Cargo.
+# It is not intended for manual editing.
+version = 4
+

Quoted 4 lines

rust/Cargo.lock new file +4 -0

+# This file is automatically @generated by Cargo.
+# It is not intended for manual editing.
+version = 4
+

rust/Cargo.lock line 14

Quoted 10 lines

rust/Cargo.lock new file +10 -0

+[[package]]
+name = "interop"
+version = "0.1.0"
+
+[[package]]
+name = "xdiff"
+version = "0.1.0"
+dependencies = [
+ "interop",
+]

I would prefer that we not check in Cargo.lock in Git. Part of the reason is that it changes across versions and so building with a different version of the toolchain can update the file.

In addition, as I mentioned downthread, because our intention is to support the Debian stable toolchain for a year after the new stable release, unless we are exceptionally careful about dependencies, we may end up with a case where distros need to use older dependencies patched for security but other users may want to update the versions to newer dependencies with security fixes but that do not work on our pinned Rust version. We can't possibly satisfy both sets of people if we pin dependencies in Cargo.lock, so we probably want to avoid checking it in and ignore it instead.

In addition, as I mentioned downthread, because our intention is to support the Debian stable toolchain for a year after the new stable release, unless we are exceptionally careful about dependencies, we may end up with a case where distros need to use older dependencies patched for security but other users may want to update the versions to newer dependencies with security fixes but that do not work on our pinned Rust version.

Quoted 16 lines · Ezekiel Newren via GitGitGadget

rust/Cargo.lock new file +10 -0

new file mode 100644
index 000000000000..fb1eac690b39
@@ -0,0 +1,14 @@
+# This file is automatically @generated by Cargo.
+# It is not intended for manual editing.
+version = 4
+
+[[package]]
+name = "interop"
+version = "0.1.0"
+
+[[package]]
+name = "xdiff"

rust/Cargo.lock line 14

Quoted 4 lines

rust/Cargo.lock new file +4 -0

+version = "0.1.0"
+dependencies = [
+ "interop",
+]

I would prefer that we not check in Cargo.lock in Git. Part of the reason is that it changes across versions and so building with a different version of the toolchain can update the file.

In addition, as I mentioned downthread, because our intention is to support the Debian stable toolchain for a year after the new stable release, unless we are exceptionally careful about dependencies, we may end up with a case where distros need to use older dependencies patched for security but other users may want to update the versions to newer dependencies with security fixes but that do not work on our pinned Rust version. We can't possibly satisfy both sets of people if we pin dependencies in Cargo.lock, so we probably want to avoid checking it in and ignore it instead.

--
brian m. carlson (they/them)
Toronto, Ontario, CA

This goes against what I think is best practices. Don’t we need
Cargo.lock to audit and debug platform specific issues, and to ensure reproducibility? Without Cargo.lock, we might get different results
one minute to the next if one of our dependencies releases a new
version. Checking in Cargo.lock aligns with Cargo’s documented best practices (https://doc.rust-lang.org/cargo/faq.html#why-have-cargolock-in-version-control).

I understand your concern and I agree that this could become a problem. I’m totally flexible on which rust version should be used, but without Cargo.lock checked in we lose the ability to audit why a build failed. I think that this will be a pain point, but numbing that pain means we can’t solve intermittent problems due to dependencies in the future.

I was one of the maintainers for Git LFS for several years. We routinely had people come to us and say, "This dependency you're using has a portion that you're not using, which has a CVE. I demand you update it and do a new release immediately because our security scanner is going off and our company policy is that there be no exceptions." This happens literally all the time and I absolutely in no case want to see those people on this list or the security list.

So the options as I see them are (a) we don't check in Cargo.lock, (b) we convince the Rust project and the ecosystem to provide LTS releases with security fixes, or (c) we only accept dependencies that have our same lifetime policy (which are very few and far between). I know this makes builds unreproducible (although not under the Reproducible Builds project's definitions), but we really don't have many alternatives.

Quoted 6 lines · Ezekiel Newren

This goes against what I think is best practices. Don’t we need
Cargo.lock to audit and debug platform specific issues, and to ensure reproducibility? Without Cargo.lock, we might get different results
one minute to the next if one of our dependencies releases a new
version. Checking in Cargo.lock aligns with Cargo’s documented best practices (https://doc.rust-lang.org/cargo/faq.html#why-have-cargolock-in-version-control).

I appreciate that, but best practices also don't limit software to a six-week lifespan. Rust the language is a great tool, but we also have a special case here in that we need to support software that upstream does not and that we care about OS distros, which upstream does not.

Note that when someone builds locally, a Cargo.lock will be created and they will get reproducible builds from that point on. It is only on first build that they will get whatever's the latest.

Quoted 6 lines

I understand your concern and I agree that this could become a problem. I’m totally flexible on which rust version should be used, but without Cargo.lock checked in we lose the ability to audit why a build failed. I think that this will be a pain point, but numbing that pain means we can’t solve intermittent problems due to dependencies in the future.

I was one of the maintainers for Git LFS for several years. We routinely had people come to us and say, "This dependency you're using has a portion that you're not using, which has a CVE. I demand you update it and do a new release immediately because our security scanner is going off and our company policy is that there be no exceptions." This happens literally all the time and I absolutely in no case want to see those people on this list or the security list.

So the options as I see them are (a) we don't check in Cargo.lock, (b) we convince the Rust project and the ecosystem to provide LTS releases with security fixes, or (c) we only accept dependencies that have our same lifetime policy (which are very few and far between). I know this makes builds unreproducible (although not under the Reproducible Builds project's definitions), but we really don't have many alternatives. --
brian m. carlson (they/them)
Toronto, Ontario, CA

I like having the Cargo.lock file to figure out why a build worked on one system, but not another. After talking with Elijah I've decided that a good solution would be to add Cargo.lock to .gitignore and change the github workflows to ensure that Cargo.lock is preserved for all builds. We should also add a comment to Cargo.toml stating that any build or test issues should include the Cargo.lock that was generated when asking for help. What does the community think of this solution?

Quoted 16 lines · Ezekiel Newren via GitGitGadget

rust/Cargo.lock new file +10 -0

new file mode 100644
index 000000000000..fb1eac690b39
@@ -0,0 +1,14 @@
+# This file is automatically @generated by Cargo.
+# It is not intended for manual editing.
+version = 4
+
+[[package]]
+name = "interop"
+version = "0.1.0"
+
+[[package]]
+name = "xdiff"

rust/Cargo.lock line 14

Quoted 4 lines

rust/Cargo.lock new file +4 -0

+version = "0.1.0"
+dependencies = [
+ "interop",
+]

I would prefer that we not check in Cargo.lock in Git. Part of the reason is that it changes across versions and so building with a different version of the toolchain can update the file.

Quoted 3 lines · brian m. carlson

I would prefer that we not check in Cargo.lock in Git. Part of the reason is that it changes across versions and so building with a different version of the toolchain can update the file.

That actually doesn't happen unless the file needs to be updated for some reason, like Cargo.toml having new dependencies or cargo update being run.

From: Ezekiel Newren <[email protected]>

Upcoming patches will accelerate and simplify xdiff, while also porting parts of it to Rust. In preparation, add some stubs and setup the Rust build. For now, it is easier to let cargo build rust and have make or meson merely link against the static library that cargo builds. In line with ongoing libification efforts, use multiple crates to allow more modularity on the Rust side. xdiff is the crate that this series will focus on, but we also introduce the interop crate for future patch series.

In order to facilitate interoperability between C and Rust, introduce C definitions for Rust primitive types in git-compat-util.h.

Makefile +2 -0

 REFTABLE_LIB = reftable/libreftable.a
 
 GENERATED_H += command-list.h

@@ -1392,6 +1397,8 @@ UNIT_TEST_OBJS += $(UNIT_TEST_DIR)/lib-reftable.o
 GITLIBS = common-main.o $(LIB_FILE) $(XDIFF_LIB) $(REFTABLE_LIB) $(LIB_FILE)
 EXTLIBS =
 
+GITLIBS += $(RUST_LIB)
+
 GIT_USER_AGENT = git/$(GIT_VERSION)
 
 ifeq ($(wildcard sha1collisiondetection/lib/sha1.h),sha1collisiondetection/lib/sha1.h)

git-compat-util.h +7 -0

index 4678e21c4cb8..82dc99764ac0 100644
@@ -196,6 +196,23 @@ static inline int is_xplatform_dir_sep(int c)
 #include "compat/msvc.h"
 #endif
 
+/* rust types */
+typedef uint8_t   u8;
+typedef uint16_t  u16;
+typedef uint32_t  u32;
+typedef uint64_t  u64;
+
+typedef int8_t    i8;

---
 xdiff/xprepare.c | 116 ++++++++++++++++++++---------------------------
 1 file changed, 50 insertions(+), 66 deletions(-)

From: Ezekiel Newren <[email protected]>

A few commits ago, we added definitions for Rust primitive types, to facilitate interoperability between C and Rust. Switch a few variables to use these types. Which, for now, will require adding some casts.

---
 xdiff/xdiffi.c    |  8 ++++----
 xdiff/xemit.c     |  2 +-
 xdiff/xmerge.c    | 14 +++++++-------
 xdiff/xpatience.c |  2 +-
 xdiff/xprepare.c  |  6 +++---
 xdiff/xtypes.h    |  6 +++---
 xdiff/xutils.c    |  4 ++--
 7 files changed, 21 insertions(+), 21 deletions(-)

Quoted 6 lines · Ezekiel Newren via GitGitGadget

From: Ezekiel Newren <[email protected]>

A few commits ago, we added definitions for Rust primitive types, to facilitate interoperability between C and Rust. Switch a few variables to use these types. Which, for now, will require adding some casts.

Hmm, interesting. I am not super familiar with how people typically handle interoperability between C and Rust, but having to change types on the C side to make it work with Rust is a bit surprising to me.

I would have expected that the Rust side would have declared its types using libc::c_int, libc::size_t, and so on. I think I have a vague preference towards putting the burden of casting on the Rust side, but, again, I am not super familiar with how transitions like these are typically approached.

Quoted 12 lines · Taylor Blau

Quoted 6 lines · Ezekiel Newren via GitGitGadget

From: Ezekiel Newren <[email protected]>

A few commits ago, we added definitions for Rust primitive types, to facilitate interoperability between C and Rust. Switch a few variables to use these types. Which, for now, will require adding some casts.

Hmm, interesting. I am not super familiar with how people typically handle interoperability between C and Rust, but having to change types on the C side to make it work with Rust is a bit surprising to me.

Quoted 5 lines

I would have expected that the Rust side would have declared its types using libc::c_int, libc::size_t, and so on. I think I have a vague preference towards putting the burden of casting on the Rust side, but, again, I am not super familiar with how transitions like these are typically approached.

Rust normally handles byte strings as slices or vectors of u8 (that is, C's uint8_t). C handles them as char, which may or may not be unsigned, as we all know, which leads to some "entertaining" problems from time to time.

Also, in general, Rust doesn't offer generic system-specific types, such as long, except for C FFI. This is actually a strong benefit, since it means we're not inclined to write unsigned long and then wonder why things are broken on Windows: instead, we write either usize (the equivalent of size_t) or u64 (for things like file sizes). This is much more ingrained than it is in Go, which has a tendency to use int (Rust's isize) a lot and much less often specific types.

If we're going to move this code entirely into Rust, then it makes sense to cast temporarily, and I'm fine doing that in C, since it's C that has the weird system-dependent behaviour (arbitrary decisions on the signedness of char). That actually allows us to have more confidence in the safety and maintainability of the Rust code since it is less system dependent and leave the suspect pieces in C. It may also, interestingly enough, also allow us to easily get rid of the weird 2 GB limit on diffs due to the unpleasant dependency on int in the xdiff code, which I would absolutely love to see.

However, I'm not dead set against casting in Rust if that's what everyone else wants instead.

Quoted 2 lines

However, I'm not dead set against casting in Rust if that's what everyone else wants instead.

In general, I too would prefer to do the casting on the C side; after all, part of the reason for Rust is the language safety, which we compromise if we force it into using ambiguously sized variables.

However, I think it might be somewhat case-dependent...

Here, we have C calling into APIs that will be defined and implemented in Rust. Further along the road of adopting Rust in more places, we may have future cases where we have Rust calling into APIs defined and implemented in C. I'm wondering if in such a world the rule of thumb that makes the most sense would be to have a caller-must-cast-as-necessary guideline, rather than specifying the casting side by language.

Quoted 12 lines · Taylor Blau

Quoted 6 lines · Ezekiel Newren via GitGitGadget

From: Ezekiel Newren <[email protected]>

A few commits ago, we added definitions for Rust primitive types, to facilitate interoperability between C and Rust. Switch a few variables to use these types. Which, for now, will require adding some casts.

Hmm, interesting. I am not super familiar with how people typically handle interoperability between C and Rust, but having to change types on the C side to make it work with Rust is a bit surprising to me.

Quoted 5 lines

I would have expected that the Rust side would have declared its types using libc::c_int, libc::size_t, and so on. I think I have a vague preference towards putting the burden of casting on the Rust side, but, again, I am not super familiar with how transitions like these are typically approached.

Rust normally handles byte strings as slices or vectors of u8 (that is, C's uint8_t). C handles them as char, which may or may not be unsigned, as we all know, which leads to some "entertaining" problems from time to time.

Also, in general, Rust doesn't offer generic system-specific types, such as long, except for C FFI. This is actually a strong benefit, since it means we're not inclined to write unsigned long and then wonder why things are broken on Windows: instead, we write either usize (the equivalent of size_t) or u64 (for things like file sizes). This is much more ingrained than it is in Go, which has a tendency to use int (Rust's isize) a lot and much less often specific types.

If we're going to move this code entirely into Rust, then it makes sense to cast temporarily, and I'm fine doing that in C, since it's C that has the weird system-dependent behaviour (arbitrary decisions on the signedness of char). That actually allows us to have more confidence in the safety and maintainability of the Rust code since it is less system dependent and leave the suspect pieces in C. It may also, interestingly enough, also allow us to easily get rid of the weird 2 GB limit on diffs due to the unpleasant dependency on int in the xdiff code, which I would absolutely love to see.

From: Ezekiel Newren <[email protected]>

A few commits ago, we added definitions for Rust primitive types, to facilitate interoperability between C and Rust. Switch a few variables to use these types. Which, for now, will require adding some casts.

xdiff/xdiffi.c +3 -3

  		char c = rec->ptr[i];
 
  		if (!XDL_ISSPACE(c))

@@ -1005,11 +1005,11 @@ static void xdl_mark_ignorable_lines(xdchange_t *xscr, xdfenv_t *xe, long flags)
 
  		rec = &xe->xdf1.recs[xch->i1];
  		for (i = 0; i < xch->chg1 && ignore; i++)
-			ignore = xdl_blankline(rec[i]->ptr, rec[i]->size, flags);
+			ignore = xdl_blankline((const char*) rec[i]->ptr, rec[i]->size, flags);
 
  		rec = &xe->xdf2.recs[xch->i2];
  		for (i = 0; i < xch->chg2 && ignore; i++)
-			ignore = xdl_blankline(rec[i]->ptr, rec[i]->size, flags);
+			ignore = xdl_blankline((const char*)rec[i]->ptr, rec[i]->size, flags);
 
  		xch->ignore = ignore;
  	}

@@ -1020,7 +1020,7 @@ static int record_matches_regex(xrecord_t *rec, xpparam_t const *xpp) {
  	size_t i;
 
  	for (i = 0; i < xpp->ignore_regex_nr; i++)
-		if (!regexec_buf(xpp->ignore_regex[i], rec->ptr, rec->size, 1,
+		if (!regexec_buf(xpp->ignore_regex[i], (const char*) rec->ptr, rec->size, 1,
  				 &regmatch, 0))
  			return 1;
 

xdiff/xemit.c +1 -1

index 1d40c9cb4076..bbf7b7f8c862 100644
@@ -24,7 +24,7 @@
 
  static long xdl_get_rec(xdfile_t *xdf, long ri, char const **rec) {
 
-	*rec = xdf->recs[ri]->ptr;
+	*rec = (char const*) xdf->recs[ri]->ptr;
 
  	return xdf->recs[ri]->size;
  }

xdiff/xmerge.c +7 -7

index af40c88a5b36..6fa6ea61a208 100644
@@ -101,8 +101,8 @@ static int xdl_merge_cmp_lines(xdfenv_t *xe1, int i1, xdfenv_t *xe2, int i2,
  	xrecord_t **rec2 = xe2->xdf2.recs + i2;
 
  	for (i = 0; i < line_count; i++) {
-		int result = xdl_recmatch(rec1[i]->ptr, rec1[i]->size,
-			rec2[i]->ptr, rec2[i]->size, flags);
+		int result = xdl_recmatch((const char*) rec1[i]->ptr, rec1[i]->size,
+			(const char*) rec2[i]->ptr, rec2[i]->size, flags);
  		if (!result)
  			return -1;
  	}

@@ -324,8 +324,8 @@ static int xdl_fill_merge_buffer(xdfenv_t *xe1, const char *name1,
 
  static int recmatch(xrecord_t *rec1, xrecord_t *rec2, unsigned long flags)
  {
-	return xdl_recmatch(rec1->ptr, rec1->size,
-			    rec2->ptr, rec2->size, flags);
+	return xdl_recmatch((char const*) rec1->ptr, rec1->size,
+			    (char const*) rec2->ptr, rec2->size, flags);
  }
 
  /*

@@ -383,10 +383,10 @@ static int xdl_refine_conflicts(xdfenv_t *xe1, xdfenv_t *xe2, xdmerge_t *m,
  		 */
  		t1.ptr = (char *)xe1->xdf2.recs[m->i1]->ptr;
  		t1.size = xe1->xdf2.recs[m->i1 + m->chg1 - 1]->ptr
-			+ xe1->xdf2.recs[m->i1 + m->chg1 - 1]->size - t1.ptr;
+			+ xe1->xdf2.recs[m->i1 + m->chg1 - 1]->size - (u8 const*) t1.ptr;
  		t2.ptr = (char *)xe2->xdf2.recs[m->i2]->ptr;
  		t2.size = xe2->xdf2.recs[m->i2 + m->chg2 - 1]->ptr
-			+ xe2->xdf2.recs[m->i2 + m->chg2 - 1]->size - t2.ptr;
+			+ xe2->xdf2.recs[m->i2 + m->chg2 - 1]->size - (u8 const*) t2.ptr;
  		if (xdl_do_diff(&t1, &t2, xpp, &xe) < 0)
  			return -1;
  		if (xdl_change_compact(&xe.xdf1, &xe.xdf2, xpp->flags) < 0 ||

@@ -440,7 +440,7 @@ static int line_contains_alnum(const char *ptr, long size)
  static int lines_contain_alnum(xdfenv_t *xe, int i, int chg)
  {
  	for (; chg; chg--, i++)
-		if (line_contains_alnum(xe->xdf2.recs[i]->ptr,
+		if (line_contains_alnum((char const*) xe->xdf2.recs[i]->ptr,
  				xe->xdf2.recs[i]->size))
  			return 1;
  	return 0;

xdiff/xpatience.c +1 -1

index 77dc411d1937..986a3a3f749a 100644
@@ -121,7 +121,7 @@ static void insert_record(xpparam_t const *xpp, int line, struct hashmap *map,
  		return;
  	map->entries[index].line1 = line;
  	map->entries[index].hash = record->ha;
-	map->entries[index].anchor = is_anchor(xpp, map->env->xdf1.recs[line - 1]->ptr);
+	map->entries[index].anchor = is_anchor(xpp, (const char*) map->env->xdf1.recs[line - 1]->ptr);
  	if (!map->first)
  		map->first = map->entries + index;
  	if (map->last) {

xdiff/xprepare.c +3 -3

index ad356281f939..747268e4fdf7 100644
@@ -96,12 +96,12 @@ static int xdl_classify_record(unsigned int pass, xdlclassifier_t *cf, xrecord_t
  	char const *line;
  	xdlclass_t *rcrec;
 
-	line = rec->ptr;
+	line = (char const*) rec->ptr;
  	hi = (long) XDL_HASHLONG(rec->ha, cf->hbits);
  	for (rcrec = cf->rchash[hi]; rcrec; rcrec = rcrec->next)
  		if (rcrec->ha == rec->ha &&
  				xdl_recmatch(rcrec->line, rcrec->size,
-					rec->ptr, rec->size, cf->flags))
+					(const char*) rec->ptr, rec->size, cf->flags))
  			break;
 
  	if (!rcrec) {

@@ -159,7 +159,7 @@ static int xdl_prepare_ctx(unsigned int pass, mmfile_t *mf, long narec, xpparam_
  				goto abort;
  			if (!(crec = xdl_cha_alloc(&xdf->rcha)))
  				goto abort;
-			crec->ptr = prev;
+			crec->ptr = (u8 const*) prev;
  			crec->size = (long) (cur - prev);
  			crec->ha = hav;
  			recs[nrec++] = crec;

xdiff/xtypes.h +3 -3

index 8b8467360ecf..6e5f67ebf380 100644
@@ -39,9 +39,9 @@ typedef struct s_chastore {
  } chastore_t;
 
  typedef struct s_xrecord {
-	char const *ptr;
-	long size;
-	unsigned long ha;
+	u8 const* ptr;
+	usize size;
+	u64 ha;
  } xrecord_t;
 
  typedef struct s_xdfile {

xdiff/xutils.c +2 -2

index 444a108f87c0..10e4f20b7c31 100644
@@ -418,10 +418,10 @@ int xdl_fall_back_diff(xdfenv_t *diff_env, xpparam_t const *xpp,
 
  	subfile1.ptr = (char *)diff_env->xdf1.recs[line1 - 1]->ptr;
  	subfile1.size = diff_env->xdf1.recs[line1 + count1 - 2]->ptr +
-		diff_env->xdf1.recs[line1 + count1 - 2]->size - subfile1.ptr;
+		diff_env->xdf1.recs[line1 + count1 - 2]->size - (u8 const*) subfile1.ptr;
  	subfile2.ptr = (char *)diff_env->xdf2.recs[line2 - 1]->ptr;
  	subfile2.size = diff_env->xdf2.recs[line2 + count2 - 2]->ptr +
-		diff_env->xdf2.recs[line2 + count2 - 2]->size - subfile2.ptr;
+		diff_env->xdf2.recs[line2 + count2 - 2]->size - (u8 const*) subfile2.ptr;
  	if (xdl_do_diff(&subfile1, &subfile2, xpp, &env) < 0)
  		return -1;
 

Hi Ezekiel

Quoted 2 lines · Ezekiel Newren via GitGitGadget

From: Ezekiel Newren <[email protected]>

Quoted 4 lines

A few commits ago, we added definitions for Rust primitive types, to facilitate interoperability between C and Rust. Switch a few variables to use these types. Which, for now, will require adding some casts.

How necessary is it to change char' to 'u8' so long as the rust and C sides both use a type that is the same size? Also what's the advantage of using these typedefs rather than the normal C types like unit8_t ?

Quoted 8 lines

xdiff/xdiffi.c +0 -0

index 5a96e36dfbea..3b364c61f671 100644
@@ -418,7 +418,7 @@ static int get_indent(xrecord_t *rec)
      long i;
      int ret = 0;
 

xdiff/xdiffi.c static int get_indent(xrecord_t *rec) · line 421

Quoted 2 lines

xdiff/xdiffi.c +1 -1

-     for (i = 0; i < rec->size; i++) {
+     for (i = 0; i < (long) rec->size; i++) {

i is a loop counter and array index so we can lose this cast by changeing i to size_t

Thanks

Phillip

Quoted 8 lines · Phillip Wood

Quoted 6 lines · Ezekiel Newren via GitGitGadget

From: Ezekiel Newren <[email protected]>

A few commits ago, we added definitions for Rust primitive types, to facilitate interoperability between C and Rust. Switch a few variables to use these types. Which, for now, will require adding some casts.

Quoted 3 lines

How necessary is it to change char' to 'u8' so long as the rust and C sides both use a type that is the same size? Also what's the advantage of using these typedefs rather than the normal C types like unit8_t ?

Rust defines char as 32 bits. C treats char as signed 8 bits. What git really means by char* is treat everything like a byte string, and u8 is how raw bytes are handled in Rust.

Quoted 8 lines

Quoted 8 lines

xdiff/xdiffi.c +0 -0

index 5a96e36dfbea..3b364c61f671 100644
@@ -418,7 +418,7 @@ static int get_indent(xrecord_t *rec)
       long i;
       int ret = 0;
 

Quoted 5 lines

xdiff/xdiffi.c static int get_indent(xrecord_t *rec) · line 421

Quoted 2 lines

xdiff/xdiffi.c +1 -1

-     for (i = 0; i < rec->size; i++) {
+     for (i = 0; i < (long) rec->size; i++) {

i is a loop counter and array index so we can lose this cast by changeing i to size_t

Ok, but I'm going to change the type of i to usize and stuff it inside the loop i.e. for (usize i = 0; ...

Quoted 3 lines

Thanks

Phillip

Quoted 13 lines · Ezekiel Newren

Quoted 11 lines · Phillip Wood

Quoted 6 lines · Ezekiel Newren via GitGitGadget

From: Ezekiel Newren <[email protected]>

A few commits ago, we added definitions for Rust primitive types, to facilitate interoperability between C and Rust. Switch a few variables to use these types. Which, for now, will require adding some casts.

How necessary is it to change char' to 'u8' so long as the rust and C sides both use a type that is the same size? Also what's the advantage of using these typedefs rather than the normal C types like unit8_t ?

Quoted 3 lines

Rust defines char as 32 bits. C treats char as signed 8 bits. What git really means by char* is treat everything like a byte string, and u8 is how raw bytes are handled in Rust.

Right - we need to use u8 on the rust side but I'm trying to understand why we need to change the type on the C side and why do we need typedefs like usize and u32 on the C side when we already have size_t and uint32_t?

Thanks

Phillip

On Mon, Jul 28, 2025 at 1:52 PM Phillip Wood <[email protected]> wrote:

Ah, I misunderstood the scope of your question. I could not fit an example of why this design pattern made sense into this patch series, so I'll explain with an example here:

If C defines a struct like below then it's obvious how to translate that into rust for ffi purposes. It also makes it clear that this C struct is expressly for the purpose of C <-> Rust interoperability.

struct some_struct {
     u8* ptr;
     usize length;
     u64 counter;
};

This is how that C struct needs to be defined in Rust so that it can interoperate with C, and making C use the Rust types reduces the chance of copy paste, and primitive type definition mismatch errors. #[repr(C)]

pub struct some_struct {
     ptr: *mut u8,
     length: usize,
     counter: u64,
};

Quoted 19 lines · Ezekiel Newren

On Mon, Jul 28, 2025 at 1:52 PM Phillip Wood <[email protected]> wrote:

Ah, I misunderstood the scope of your question. I could not fit an example of why this design pattern made sense into this patch series, so I'll explain with an example here:

If C defines a struct like below then it's obvious how to translate that into rust for ffi purposes. It also makes it clear that this C struct is expressly for the purpose of C <-> Rust interoperability.

struct some_struct {
     u8* ptr;
     usize length;
     u64 counter;
};

This is how that C struct needs to be defined in Rust so that it can interoperate with C, and making C use the Rust types reduces the chance of copy paste, and primitive type definition mismatch errors. #[repr(C)]

Quoted 5 lines

pub struct some_struct {
     ptr: *mut u8,
     length: usize,
     counter: u64,
};

How is the pointer, length pair used in rust? Normally one would use a slice so do we have to construct a slice every time we want to use the data in this struct, or do we copy the data in this struct into to a an idiomatic struct with a slice member? If we end up copying there doesn't seem much point in changing all the types in the C struct as we can define a rust struct using *c_char, c_long etc. to interface with the C code and covert them to an appropriate rust type when we copy the data to the idiomatic version that is then used by the rust of the rust code. I can see the value of the typedefs for documenting C<->rust interop if the same struct is used by both but if we end up copying data on the rust side I'm not so sure.

Thanks

Phillip

Quoted 39 lines · Phillip Wood

Quoted 19 lines · Ezekiel Newren

On Mon, Jul 28, 2025 at 1:52 PM Phillip Wood <[email protected]> wrote:

Ah, I misunderstood the scope of your question. I could not fit an example of why this design pattern made sense into this patch series, so I'll explain with an example here:

If C defines a struct like below then it's obvious how to translate that into rust for ffi purposes. It also makes it clear that this C struct is expressly for the purpose of C <-> Rust interoperability.

struct some_struct {
      u8* ptr;
      usize length;
      u64 counter;
};

This is how that C struct needs to be defined in Rust so that it can interoperate with C, and making C use the Rust types reduces the chance of copy paste, and primitive type definition mismatch errors. #[repr(C)]

Quoted 5 lines

pub struct some_struct {
      ptr: *mut u8,
      length: usize,
      counter: u64,
};

How is the pointer, length pair used in rust? Normally one would use a slice so do we have to construct a slice every time we want to use the data in this struct, or do we copy the data in this struct into to a an idiomatic struct with a slice member? If we end up copying there doesn't seem much point in changing all the types in the C struct as we can define a rust struct using *c_char, c_long etc. to interface with the C code and covert them to an appropriate rust type when we copy the data to the idiomatic version that is then used by the rust of the rust code. I can see the value of the typedefs for documenting C<->rust interop if the same struct is used by both but if we end up copying data on the rust side I'm not so sure.

Quoted 3 lines

Thanks

Phillip

Passing pointer + length from c to Rust does not incur a memory copy overhead. Take a look at rust/xdiff/src/lib.rs wich has the following rust function defined:

#[no_mangle]

unsafe extern "C" fn xxh3_64(ptr: *const u8, size: usize) -> u64 {
     let slice = std::slice::from_raw_parts(ptr, size);
     xxhash_rust::xxh3::xxh3_64(slice)
}

Creating a slice tells the compiler what assumptions it can make about that memory. On the C side in xdiff/xprepare.c:

extern u64 xxh3_64(u8 const* ptr, usize size);

and then it's called like this in that same file:

rec->ha = xxh3_64(rec->ptr, rec->size);

I really wanted to show my ivec type that made passing an interoperable vector type between C and Rust easy and fast, but this patch series is already getting very long.

Quoted 9 lines · Ezekiel Newren

Quoted 8 lines · Phillip Wood

Quoted 6 lines · Ezekiel Newren via GitGitGadget

From: Ezekiel Newren <[email protected]>

A few commits ago, we added definitions for Rust primitive types, to facilitate interoperability between C and Rust. Switch a few variables to use these types. Which, for now, will require adding some casts.

Quoted 7 lines

Quoted 3 lines

How necessary is it to change char' to 'u8' so long as the rust and C sides both use a type that is the same size? Also what's the advantage of using these typedefs rather than the normal C types like unit8_t ?

Rust defines char as 32 bits. C treats char as signed 8 bits. What git
really means by char* is treat everything like a byte string, and u8 is how raw bytes are handled in Rust.

Right - we need to use u8 on the rust side but I'm trying to understand why we need to change the type on the C side and why do we need typedefs like usize and u32 on the C side when we already have size_t and uint32_t?

Rust defines char as 32 bits. C treats char as signed 8 bits. What git really means by char* is treat everything like a byte string, and u8 is how raw bytes are handled in Rust.

xdiff/xtypes.h +0 -0

index 8b8467360ecf..6e5f67ebf380 100644
@@ -39,9 +39,9 @@ typedef struct s_chastore {
 } chastore_t;
 

---
 xdiff/xprepare.c | 75 ++++++++++++++++++++++++++++--------------------
 1 file changed, 44 insertions(+), 31 deletions(-)

From: Ezekiel Newren <[email protected]>

We want to use xxhash for faster hashing. To facilitate that and to simplify the code. Separate the concerns of parsing and hashing into discrete steps. This makes swapping the hash function much easier. Since xdl_hash_record() both parses and hashses lines, this requires some slight code restructuring.

Signed-off-by Ezekiel Newren <[email protected]>

---
  xdiff/xprepare.c | 75 ++++++++++++++++++++++++++++--------------------
  1 file changed, 44 insertions(+), 31 deletions(-)

xdiff/xprepare.c +0 -0

index 747268e4fdf7..c44005e9bbb8 100644

xdiff/xprepare.c +11 -20

+		rec->ptr = ptr;
+		rec->size = length;
+		rec->ha = 0;
+		xdf->recs[xdf->nrec++] = rec;
+		ptr += length;
+		len -= length;
+	}
+
+}
+
+
  static int xdl_prepare_ctx(unsigned int pass, mmfile_t *mf, long narec, xpparam_t const *xpp,
  			   xdlclassifier_t *cf, xdfile_t *xdf) {
-	long nrec, bsize;
-	unsigned long hav;
-	char const *blk, *cur, *top, *prev;
-	xrecord_t *crec;
-	xrecord_t **recs;
  	unsigned long *ha;
  	char *rchg;
  	long *rindex;

@@ -143,50 +169,37 @@ static int xdl_prepare_ctx(unsigned int pass, mmfile_t *mf, long narec, xpparam_
  	ha = NULL;
  	rindex = NULL;
  	rchg = NULL;
-	recs = NULL;
 
  	if (xdl_cha_init(&xdf->rcha, sizeof(xrecord_t), narec / 4 + 1) < 0)
  		goto abort;
-	if (!XDL_ALLOC_ARRAY(recs, narec))
-		goto abort;
 
-	nrec = 0;
-	if ((cur = blk = xdl_mmfile_first(mf, &bsize))) {
-		for (top = blk + bsize; cur < top; ) {
-			prev = cur;
-			hav = xdl_hash_record(&cur, top, xpp->flags);
-			if (XDL_ALLOC_GROW(recs, nrec + 1, narec))
-				goto abort;
-			if (!(crec = xdl_cha_alloc(&xdf->rcha)))
-				goto abort;
-			crec->ptr = (u8 const*) prev;
-			crec->size = (long) (cur - prev);
-			crec->ha = hav;

xdiff/xprepare.c +6 -7

  	}
 
-	if (!XDL_CALLOC_ARRAY(rchg, nrec + 2))
+
+	if (!XDL_CALLOC_ARRAY(rchg, xdf->nrec + 2))
  		goto abort;
 
  	if ((XDF_DIFF_ALG(xpp->flags) != XDF_PATIENCE_DIFF) &&
  	    (XDF_DIFF_ALG(xpp->flags) != XDF_HISTOGRAM_DIFF)) {
-		if (!XDL_ALLOC_ARRAY(rindex, nrec + 1))
+		if (!XDL_ALLOC_ARRAY(rindex, xdf->nrec + 1))
  			goto abort;
-		if (!XDL_ALLOC_ARRAY(ha, nrec + 1))
+		if (!XDL_ALLOC_ARRAY(ha, xdf->nrec + 1))
  			goto abort;
  	}
 
-	xdf->nrec = nrec;
-	xdf->recs = recs;
  	xdf->rchg = rchg + 1;
  	xdf->rindex = rindex;
  	xdf->nreff = 0;
  	xdf->ha = ha;
  	xdf->dstart = 0;
-	xdf->dend = nrec - 1;
+	xdf->dend = xdf->nrec - 1;
 
  	return 0;
 

@@ -194,7 +207,7 @@ abort:
  	xdl_free(ha);
  	xdl_free(rindex);
  	xdl_free(rchg);
-	xdl_free(recs);
+	xdl_free(xdf->recs);
  	xdl_cha_free(&xdf->rcha);
  	return -1;
  }

    // desktop
    // CPU: 8-core Intel Core i7-9700 (-MCP-) speed/min/max: 800/800/4700 MHz
    $ hyperfine --warmup 3 -L exe $BASE/build_release/git,$BASE/build_v2.49.0/git '{exe} log --oneline --shortstat v6.8..v6.9 >/dev/null'
    Benchmark 1: /home/steamuser/dev/git/build_release/git log --oneline --shortstat v6.8..v6.9 >/dev/null
      Time (mean ± σ):      6.823 s ±  0.020 s    [User: 6.624 s, System: 0.180 s]
      Range (min … max):    6.801 s …  6.858 s    10 runs

    Benchmark 2: /home/steamuser/dev/git/build_v2.49.0/git log --oneline --shortstat v6.8..v6.9 >/dev/null
      Time (mean ± σ):      8.151 s ±  0.024 s    [User: 7.928 s, System: 0.198 s]
      Range (min … max):    8.105 s …  8.184 s    10 runs

    Summary
      /home/steamuser/dev/git/build_release/git log --oneline --shortstat v6.8..v6.9 >/dev/null ran
        1.19 ± 0.01 times faster than /home/steamuser/dev/git/build_v2.49.0/git log --oneline --shortstat v6.8..v6.9 >/dev/null

Signed-off-by Ezekiel Newren <[email protected]>

---
 rust/Cargo.lock       |  7 +++++++
 rust/xdiff/Cargo.toml |  1 +
 rust/xdiff/src/lib.rs |  7 +++++++
 xdiff/xprepare.c      | 19 +++++++++++++++++--
 4 files changed, 32 insertions(+), 2 deletions(-)

xdiff/xprepare.c +7 -0

+		for (usize i = 0; i < (usize) xdf->nrec; i++) {
+			xrecord_t *rec = xdf->recs[i];
+			rec->ha = xxh3_64(rec->ptr, rec->size);
+		}
+	} else {
+		for (usize i = 0; i < (usize) xdf->nrec; i++) {
+			xrecord_t *rec = xdf->recs[i];

xdiff/xprepare.c +5 -0

+extern u64 xxh3_64(u8 const* ptr, usize size);
+
+
 static int xdl_prepare_ctx(unsigned int pass, mmfile_t *mf, long narec, xpparam_t const *xpp,
 			   xdlclassifier_t *cf, xdfile_t *xdf) {
 	unsigned long *ha;

@@ -175,14 +178,26 @@ static int xdl_prepare_ctx(unsigned int pass, mmfile_t *mf, long narec, xpparam_
 
 	xdl_parse_lines(mf, narec, xdf);
 
+	if ((xpp->flags & XDF_WHITESPACE_FLAGS) == 0) {
+		for (usize i = 0; i < (usize) xdf->nrec; i++) {

Quoted 12 lines · Ezekiel Newren via GitGitGadget

xdiff/xprepare.c +5 -0

+extern u64 xxh3_64(u8 const* ptr, usize size);
+
+
 static int xdl_prepare_ctx(unsigned int pass, mmfile_t *mf, long narec, xpparam_t const *xpp,
                         xdlclassifier_t *cf, xdfile_t *xdf) {
      unsigned long *ha;

@@ -175,14 +178,26 @@ static int xdl_prepare_ctx(unsigned int pass, mmfile_t *mf, long narec, xpparam_
 
      xdl_parse_lines(mf, narec, xdf);
 
+     if ((xpp->flags & XDF_WHITESPACE_FLAGS) == 0) {
+             for (usize i = 0; i < (usize) xdf->nrec; i++) {

xdiff/xprepare.c static int xdl_prepare_ctx(unsigned int pass, mmfile_t *mf, long narec, xpparam_ · line 192

Quoted 10 lines

xdiff/xprepare.c +10 -0

+                     xrecord_t *rec = xdf->recs[i];
+                     rec->ha = xxh3_64(rec->ptr, rec->size);
+             }
+     } else {
+             for (usize i = 0; i < (usize) xdf->nrec; i++) {
+                     xrecord_t *rec = xdf->recs[i];
+                     char const* dump = (char const*) rec->ptr;
+                     rec->ha = xdl_hash_record(&dump, (char const*) (rec->ptr + rec->size), xpp->flags);
+             }
+     }

As a technology demonstration and proof of concept patch, this is very nice, but to be upstreamed for real, we'd want a variant of xxhash that can work with the contents with whitespace squashed to be usable with various whitespace ignoring modes of operation. When that happens, and when the result turns out to be more performant, we can lose the xdl_hash_record() and require only the xxhash, which would be great.

And that variant of xxhash that understands whitespace squashing can of course be written in Rust as a part of this effort when the series loses its RFC status. At the same time, those who want to use our xdiff code in third-party software (like libgit2 and vim) may want to reimplement it in C in their copy.

Thanks.

Quoted 34 lines · Junio C Hamano

Quoted 12 lines · Ezekiel Newren via GitGitGadget

xdiff/xprepare.c +5 -0

+extern u64 xxh3_64(u8 const* ptr, usize size);
+
+
 static int xdl_prepare_ctx(unsigned int pass, mmfile_t *mf, long narec, xpparam_t const *xpp,
                         xdlclassifier_t *cf, xdfile_t *xdf) {
      unsigned long *ha;

@@ -175,14 +178,26 @@ static int xdl_prepare_ctx(unsigned int pass, mmfile_t *mf, long narec, xpparam_
 
      xdl_parse_lines(mf, narec, xdf);
 
+     if ((xpp->flags & XDF_WHITESPACE_FLAGS) == 0) {
+             for (usize i = 0; i < (usize) xdf->nrec; i++) {

xdiff/xprepare.c static int xdl_prepare_ctx(unsigned int pass, mmfile_t *mf, long narec, xpparam_ · line 192

Quoted 10 lines

xdiff/xprepare.c +10 -0

+                     xrecord_t *rec = xdf->recs[i];
+                     rec->ha = xxh3_64(rec->ptr, rec->size);
+             }
+     } else {
+             for (usize i = 0; i < (usize) xdf->nrec; i++) {
+                     xrecord_t *rec = xdf->recs[i];
+                     char const* dump = (char const*) rec->ptr;
+                     rec->ha = xdl_hash_record(&dump, (char const*) (rec->ptr + rec->size), xpp->flags);
+             }
+     }

As a technology demonstration and proof of concept patch, this is very nice, but to be upstreamed for real, we'd want a variant of xxhash that can work with the contents with whitespace squashed to be usable with various whitespace ignoring modes of operation. When that happens, and when the result turns out to be more performant, we can lose the xdl_hash_record() and require only the xxhash, which would be great.

Quoted 7 lines

And that variant of xxhash that understands whitespace squashing can of course be written in Rust as a part of this effort when the series loses its RFC status. At the same time, those who want to use our xdiff code in third-party software (like libgit2 and vim) may want to reimplement it in C in their copy.

Thanks.

What is the git precedent for replacement code that is easier to read and maintain while also being more secure, but is slower? I think hashing with whitespace handling in Rust might fall in that category.

As far as I can tell the Rust code for dealing with whitespace is going to be slower than the C code because xdiff used a hash algorithm (DJB2a) that can operate 1 byte at a time and combined hashing with determining the length. Xxhash requires that the length be known beforehand and the memory to be contiguous or to hash it in chunks. Hashing 1 byte at a time with Xxhash is VERY slow since it's just copying to an internal buffer until a full block is ready.

On a broader note. How do I show the mailing list the changes that I've made to this branch/patch series? I'm not sure what the proper procedure is or even how to do it. What commands would I run, or web browser steps would I take to show my newest commits?

Hi Ezekiel,

rust/xdiff/src/lib.rs line 7

Quoted 12 lines · Ezekiel Newren via GitGitGadget

rust/xdiff/src/lib.rs +7 -0

index e69de29bb2d1..96975975a1ba 100644
@@ -0,0 +1,7 @@
+
+
+#[no_mangle]
+unsafe extern "C" fn xxh3_64(ptr: *const u8, size: usize) -> u64 {
+    let slice = std::slice::from_raw_parts(ptr, size);
+    xxhash_rust::xxh3::xxh3_64(slice)
+}

I know that this is a pretty small file, but I do notice that it does not have a license header.

This reminds me of the unfortunate oversight to be careful about making (and keeping) libgit.a's source files compatible with libgit2's license to nurture a fruitful exchange between those two projects.

With Rust, we still have a really good chance to learn from history and avoid that mistake: Gitoxide is a very exciting project with clear overlap in its mission to implement Git functionality in Rust. Gitoxide is dual-licensed under the Apache License v2 and the MIT license (see
https://github.com/GitoxideLabs/gitoxide?tab=readme-ov-file#license).

Would you mind adding a license header to that file that explicitly allows the contents of the file to be used in Gitoxide, to get the Rust effort started on a good foot?

Hi Johannes

Quoted 20 lines · Johannes Schindelin

Hi Ezekiel,

rust/xdiff/src/lib.rs line 7

Quoted 12 lines · Ezekiel Newren via GitGitGadget

rust/xdiff/src/lib.rs +7 -0

index e69de29bb2d1..96975975a1ba 100644
@@ -0,0 +1,7 @@
+
+
+#[no_mangle]
+unsafe extern "C" fn xxh3_64(ptr: *const u8, size: usize) -> u64 {
+    let slice = std::slice::from_raw_parts(ptr, size);
+    xxhash_rust::xxh3::xxh3_64(slice)
+}

I know that this is a pretty small file, but I do notice that it does not have a license header.

Quoted 3 lines

This reminds me of the unfortunate oversight to be careful about making (and keeping) libgit.a's source files compatible with libgit2's license to nurture a fruitful exchange between those two projects.

I'm not sure I follow your reasoning here. libgit2 was started after git and chose to use an incompatible license. I wasn't around at the time but isn't there a list of git contributors who are happy to re-license their contributions with the linking exception used by libgit2?

Quoted 9 lines

With Rust, we still have a really good chance to learn from history and avoid that mistake: Gitoxide is a very exciting project with clear overlap in its mission to implement Git functionality in Rust. Gitoxide is dual-licensed under the Apache License v2 and the MIT license (see
https://github.com/GitoxideLabs/gitoxide?tab=readme-ov-file#license).

Would you mind adding a license header to that file that explicitly allows the contents of the file to be used in Gitoxide, to get the Rust effort started on a good foot?

I wary of that for two reasons. Firstly over time it is de-facto re-licensing git as the amount of rust code grows and the amount of C code shrinks which deserves a wider discussion. Secondly it makes it harder to convert our C code which is licensed under GPL2 (or in the case of xdiff LGPL) to rust if the rust code uses a different license.

If someone wants to start a discussion about re-licensing git (and is prepared to do all of the associated admin in the event that it happens) then by all means do so but I don't think it we want to slip such a change into this series.

It would not even take more than something as simple as GPLv2+exception. We do have prior art for that: The Git project itself suggests in its very own COPYING file to use the following license in new files:

This file is licensed under the GPL v2, or a later version at the discretion of Linus.

Note the exception? For new Rust code (and of course excluding code that has been ported verbatim from GPLv2-licensed code), GPL v2 could be used with an exception along these lines: This file is licensed under the GPL v2, with the exception that it can be freely used in the Gitoxide project.

I am not a lawyer (which everybody but laywers are nowadays required to say), therefore this likely needs some tweaking.

Quoted 11 lines · Johannes Schindelin

It would not even take more than something as simple as GPLv2+exception. We do have prior art for that: The Git project itself suggests in its very own COPYING file to use the following license in new files:

This file is licensed under the GPL v2, or a later version at the discretion of Linus.

Note the exception? For new Rust code (and of course excluding code that has been ported verbatim from GPLv2-licensed code), GPL v2 could be used with an exception along these lines: This file is licensed under the GPL v2, with the exception that it can be freely used in the Gitoxide project.

I think this "and of course" parenthetical might be a sticking point. Obviously taking the code verbatim and re-licensing it is not allowed. But I think even reading the C code and then writing substantially similar Rust code may be legally questionable. The Rust code under the more permissive license has to either be clean-room, or have permission for re-licensing from the original authors (which is getting to be all but impossible over time as code ends up being touched by many people).

I think this is the same issue that libgit2 ran into. If it were just a matter of porting over and re-writing new features, more of it would have been done. But for code to come under the new license it can't just be a port, but has to be an independent work.

Thanks for putting this so clearly, I agree with everything that you've written here. Another thing I'm concerned/confused about is how the exception for a single project works in practice. Does it mean that a third party that wants to re-use some code from GitOxide has to check if the code originally came from Git to determine which license it is under? Or does it mean that anyone who wants to use Git's code without the copyleft restrictions can do so if they launder it through GitOxide first? Neither of those seems like a great outcome.

Quoted 8 lines · Phillip Wood

Thanks for putting this so clearly, I agree with everything that you've written here. Another thing I'm concerned/confused about is how the exception for a single project works in practice. Does it mean that a third party that wants to re-use some code from GitOxide has to check if the code originally came from Git to determine which license it is under? Or does it mean that anyone who wants to use Git's code without the copyleft restrictions can do so if they launder it through GitOxide first? Neither of those seems like a great outcome.

If I understand the suggestion correctly, it's not to license it specifically to GitOxide. It's to use a permissive license (like GPL with linking exception) that would make it compatible with other projects with similar licenses (like GitOxide).

Quoted 8 lines · Phillip Wood

Thanks for putting this so clearly, I agree with everything that you've written here. Another thing I'm concerned/confused about is how the exception for a single project works in practice. Does it mean that a third party that wants to re-use some code from GitOxide has to check if the code originally came from Git to determine which license it is under? Or does it mean that anyone who wants to use Git's code without the copyleft restrictions can do so if they launder it through GitOxide first? Neither of those seems like a great outcome.

If I understand the suggestion correctly, it's not to license it specifically to GitOxide. It's to use a permissive license (like GPL with linking exception) that would make it compatible with other projects with similar licenses (like GitOxide).

Quoted 4 lines

If I understand the suggestion correctly, it's not to license it specifically to GitOxide. It's to use a permissive license (like GPL with linking exception) that would make it compatible with other projects with similar licenses (like GitOxide).

I was responding to this paragraph in Johannes' message

Note the exception? For new Rust code (and of course excluding code that has been ported verbatim from GPLv2-licensed code), GPL v2 could be used with an exception along these lines: This file is licensed under the GPL v2, with the exception that it can be freely used in the Gitoxide project.

That suggestion is pretty close to what libgit2 has in its git.git-authors file[1]. I'm not sure how practical it is to special case just one project though.

Quoted 13 lines · Phillip Wood

Quoted 4 lines

If I understand the suggestion correctly, it's not to license it specifically to GitOxide. It's to use a permissive license (like GPL with linking exception) that would make it compatible with other projects with similar licenses (like GitOxide).

I was responding to this paragraph in Johannes' message

Note the exception? For new Rust code (and of course excluding code that has been ported verbatim from GPLv2-licensed code), GPL v2 could be used with an exception along these lines: This file is licensed under the GPL v2, with the exception that it can be freely used in the Gitoxide project.

Quoted 3 lines

That suggestion is pretty close to what libgit2 has in its git.git-authors file[1]. I'm not sure how practical it is to special case just one project though.

Ah, yeah, I may have misunderstood the proposal then.

I don't think that really changes much with respect to my concern, which is for existing code.

That probably needs to be explicitly granted
permission to relicense, whether to a specific project or not, and it is hard to pinpoint a definitive author for a lot of it (even if many authors have agreed to relicensing) because it has been touched by so many people over the years.

So there are probably two separate legal issues to consider:

- how the explicit re-licensing grant is worded

- what problems may came up with porting existing code that has been touched by many people

Quoted 8 lines · Jeff King

That probably needs to be explicitly granted
permission to relicense, whether to a specific project or not, and it is hard to pinpoint a definitive author for a lot of it (even if many authors have agreed to relicensing) because it has been touched by so many people over the years.

So there are probably two separate legal issues to consider:

Quoted 4 lines

- how the explicit re-licensing grant is worded

- what problems may came up with porting existing code that has been touched by many people

If we want to seriously consider this we should probably reach out to Conservancy for some advice. Did this end up being discussed at the Contributor's Summit?

Quoted 19 lines · Jeff King

Quoted 16 lines · Phillip Wood

Quoted 4 lines

If I understand the suggestion correctly, it's not to license it specifically to GitOxide. It's to use a permissive license (like GPL with linking exception) that would make it compatible with other projects with similar licenses (like GitOxide).

I was responding to this paragraph in Johannes' message

Note the exception? For new Rust code (and of course excluding code that has been ported verbatim from GPLv2-licensed code), GPL v2 could be used with an exception along these lines: This file is licensed under the GPL v2, with the exception that it can be freely used in the Gitoxide project.

That suggestion is pretty close to what libgit2 has in its git.git-authors file[1]. I'm not sure how practical it is to special case just one project though.

Quoted 4 lines

Ah, yeah, I may have misunderstood the proposal then.

I don't think that really changes much with respect to my concern, which is for existing code.

I agree it doesn't change anything with regard to that, I think it just adds more potential problems.

Hi Johannes

Quoted 20 lines · Johannes Schindelin

Hi Ezekiel,

rust/xdiff/src/lib.rs line 7

Quoted 12 lines · Ezekiel Newren via GitGitGadget

rust/xdiff/src/lib.rs +7 -0

index e69de29bb2d1..96975975a1ba 100644
@@ -0,0 +1,7 @@
+
+
+#[no_mangle]
+unsafe extern "C" fn xxh3_64(ptr: *const u8, size: usize) -> u64 {
+    let slice = std::slice::from_raw_parts(ptr, size);
+    xxhash_rust::xxh3::xxh3_64(slice)
+}

I know that this is a pretty small file, but I do notice that it does not have a license header.

Quoted 3 lines

This reminds me of the unfortunate oversight to be careful about making (and keeping) libgit.a's source files compatible with libgit2's license to nurture a fruitful exchange between those two projects.

I'm not sure I follow your reasoning here. libgit2 was started after git and chose to use an incompatible license. I wasn't around at the time but isn't there a list of git contributors who are happy to re-license their contributions with the linking exception used by libgit2?

Quoted 9 lines

With Rust, we still have a really good chance to learn from history and avoid that mistake: Gitoxide is a very exciting project with clear overlap in its mission to implement Git functionality in Rust. Gitoxide is dual-licensed under the Apache License v2 and the MIT license (see
https://github.com/GitoxideLabs/gitoxide?tab=readme-ov-file#license).

Would you mind adding a license header to that file that explicitly allows the contents of the file to be used in Gitoxide, to get the Rust effort started on a good foot?

I wary of that for two reasons. Firstly over time it is de-facto re-licensing git as the amount of rust code grows and the amount of C code shrinks which deserves a wider discussion. Secondly it makes it harder to convert our C code which is licensed under GPL2 (or in the case of xdiff LGPL) to rust if the rust code uses a different license.

If someone wants to start a discussion about re-licensing git (and is prepared to do all of the associated admin in the event that it happens) then by all means do so but I don't think it we want to slip such a change into this series.

Thanks

Phillip

.github/workflows/main.yml on: [push, pull_request] · line 7

Quoted 9 lines · Ezekiel Newren via GitGitGadget

.github/workflows/main.yml +1 -0

index 7dbf9f7f123c..8aac18a6ba45 100644
@@ -4,6 +4,7 @@ on: [push, pull_request]
 
 env:
   DEVELOPER: 1
+  RUST_VERSION: 1.87.0

Our discussed plan is to support the version in Debian stable, plus a year. So we'd be supporting 1.63.0 for a year after trixie's release.

The reason for that is that people build backports and security updates for Git for stable releases of distros and they will use the distro toolchain for doing so. Forcing distros to constantly build with the latest toolchain is pretty hostile, especially since the lifespan of Rust release is six weeks.

If the Rust project provides LTS releases in the future, then we can consider adopting those.

.github/workflows/main.yml on: [push, pull_request]

Quoted 6 lines

.github/workflows/main.yml +6 -0

+if [ "$rust_target" = "release" ]; then
+  rust_args="--release"
+  export RUSTFLAGS='-Aunused_imports -Adead_code'
+elif [ "$rust_target" = "debug" ]; then
+  rust_args=""
+  export RUSTFLAGS='-Aunused_imports -Adead_code -C debuginfo=2 -C opt-level=1 -C force-frame-pointers=yes'

Can you say a little about why these options are needed and the defaults are inadequate? For instance, I build with the default options both in my personal projects and at work and don't see a problem.

I don't know if you plan to do this in a future series, but we'd also want cargo's tests to be run as part of CI and we'd want a lint job that ran clippy with both 1.63.0 and the latest stable version of Rust to make sure things were tidy.

--
brian m. carlson (they/them)
Toronto, Ontario, CA

On Thu, Jul 17, 2025 at 3:23 PM brian m. carlson

Quoted 15 lines · <[email protected]> wrote:

Quoted 11 lines · Ezekiel Newren via GitGitGadget

.github/workflows/main.yml +0 -0

index 7dbf9f7f123c..8aac18a6ba45 100644
@@ -4,6 +4,7 @@ on: [push, pull_request]
 

env:
DEVELOPER: 1
+ RUST_VERSION: 1.87.0

Our discussed plan is to support the version in Debian stable, plus a year. So we'd be supporting 1.63.0 for a year after trixie's release.

Quoted 8 lines

The reason for that is that people build backports and security updates for Git for stable releases of distros and they will use the distro toolchain for doing so. Forcing distros to constantly build with the latest toolchain is pretty hostile, especially since the lifespan of Rust release is six weeks.

If the Rust project provides LTS releases in the future, then we can consider adopting those.

The RUST_VERSION variable in .github/workflows/main.yaml had to have a specific version. 1.87.0 was selected since that's what I was using locally. Elijah made me aware that an older version of rust might be desired, but didn't know which one. I'll switch to 1.63.0 or whatever the community decides.

Quoted 10 lines

.github/workflows/main.yml

Quoted 6 lines

.github/workflows/main.yml +6 -0

+if [ "$rust_target" = "release" ]; then
+  rust_args="--release"
+  export RUSTFLAGS='-Aunused_imports -Adead_code'
+elif [ "$rust_target" = "debug" ]; then
+  rust_args=""
+  export RUSTFLAGS='-Aunused_imports -Adead_code -C debuginfo=2 -C opt-level=1 -C force-frame-pointers=yes'

Can you say a little about why these options are needed and the defaults are inadequate? For instance, I build with the default options both in my personal projects and at work and don't see a problem.

What I found is that if I have a Rust function

#[no_mangle]
pub fn call_from_c(arg: u64) {}

which is only meant to be called from C and isn’t called from elsewhere in Rust, then cargo will misidentify this function as dead code. This was the reason for adding ‘-Adead_code’.

The reason for adding ‘-Aunused_imports’ is somewhat IDE related; if I paste code somewhere, RustRover will sometimes automatically add the necessary imports. However, if I delete a chunk of code, it’ll highlight the imports that are no longer used if I scroll to the top of the file, but it won’t automatically remove them. Since they aren’t automatically removed, it’s easier to build with ‘-Aunused_imports’.

This series accelerates xdiff by 5-19%.

It also introduces Rust as a hard dependency.

…and it doesn’t yet pass a couple of the github workflows; hints from Windows experts, and opinions on ambiguous primitives would be appreciated (see below).

This is just the beginning of many patches that I have to convert portions of, maybe eventually all of, xdiff to Rust. While working on that conversion, I found several ways to clarify the code, along with some optimizations.

So...

This obviously raises the question of whether we are ready to accept a hard dependency on Rust. Previous discussions on the mailing list and at Git Merge 2024 have not answered that question. If not now, will we be willing to accept such a hard dependency later? And what route do we want to take to get there?

Quoted 1 line · Ezekiel Newren via GitGitGadget

This series accelerates xdiff by 5-19%.

That's great.

Quoted 1 line

It also introduces Rust as a hard dependency.

I think that's fine. We already discussed doing this at the last Contributor Summit in Berlin and everyone was in favour. While we did not have every contributor represented, I think that unanimity of the contributors present is a compelling enough reason.

I realize that that means that we will lose support for some platforms. I ultimately think that it's up to the porters and maintainers for a platform to maintain appropriate toolchains on that platform and that, while we should be cognizant of the requirements for adding new platforms or architectures, that shouldn't prevent the inclusion of important new tools like memory-safe languages.

I would like to see a change to our platform policy and a policy on Rust before we merge this. I know your series adds support for 1.87, but because distros don't run the latest toolchain, we had discussed in the past targeting Debian stable's version plus an additional year after the new release. This means we'll support a Rust version for three years, which is a reasonable amount of time for a toolchain and allows distros to easily backport security fixes.

If you would like, you are welcome to use my proposed policy as a basis for this, or I can send that out as a separate document if you don't want to write one or revise mine.

I agree. I don't think that there is ever going to be a "perfect" time to introduce a hard dependency on Rust, and I don't think that should hold the project back from adopting it.

I am far from a Rust expert, but I think that a more modern, memory-safe language will attract newer contributors who may have a fresher perspective on the project, and I think that's a good thing.

The alternative, of course, is to continue to use C and not take any dependency on Rust. I think there is a middle-ground in there somewhere to be able to build with (e.g.) "make" or "make RUST=1", but I would really like to see the project take a firmer stance here.

I worry that having build support for both "with Rust" and "C only" will create a headache not just at the build system level, but also in the code itself. Having a patchwork of features, optimizations, or bug fixes that either are or aren't supported depending on whether Rust support was specified at build-time seems like a worst-of-all-worlds outcome.

Agreed. Of course, I think we would all like Git to be able to build and run on as many platforms as is reasonably possible. But we cannot support all platforms for all time. It is also not the Git project's responsibility to ensure that every platform is Rust-friendly.

Hopefully the platforms that we currently support but won't after this patch series have niche enough workloads that they do not need the absolute latest-and-greatest Git release at all times.

Yeah, I think that this is the most interesting part of the discussion here. I am not knowledgeable enough about Rust's release cadence and platform compatibility to have an opinion here. But I trust brian's judgement ;-).

Quoted 7 lines · Taylor Blau

I agree. I don't think that there is ever going to be a "perfect" time to introduce a hard dependency on Rust, and I don't think that should hold the project back from adopting it.

I am far from a Rust expert, but I think that a more modern, memory-safe language will attract newer contributors who may have a fresher perspective on the project, and I think that's a good thing.

Yes, I think that's true. Rust is by far the most admired programming language to work with, according to the 2024 Stack Overflow Developer Survey. We will likely attract new contributors who find C intimidating or a bit of a hassle[0] but are excited about working on Rust, especially in a project as compelling as Git[1].

Quoted 10 lines

The alternative, of course, is to continue to use C and not take any dependency on Rust. I think there is a middle-ground in there somewhere to be able to build with (e.g.) "make" or "make RUST=1", but I would really like to see the project take a firmer stance here.

I worry that having build support for both "with Rust" and "C only" will create a headache not just at the build system level, but also in the code itself. Having a patchwork of features, optimizations, or bug fixes that either are or aren't supported depending on whether Rust support was specified at build-time seems like a worst-of-all-worlds outcome.

I definitely agree. I already find it terribly inconvenient when I end up when git grep doesn't support -P and I imagine that having lots of features that weren't available would be bothersome.

I also think that using a combination of C and Rust will end up with us still writing a lot of unsafe Rust code to interoperate with C. If we want to reap the benefits in terms of memory and thread safety[2], we'll be better off sticking with just Rust.

I will also say that while it may be more challenging to compile Git at first on Windows, as we move more towards an all-Rust codebase, Git may end up being easier to maintain there as we depend more on the standard library.

Fully agreed. I've said so at the last contributors summit, but I think it would become awfully unmaintainable if we retain two implementations of every subsystem that we convert to Rust. If we decide to use Rust I would strongly advocate for going all-in.

I am far from a Rust expert, but I think that a more modern, memory-safe language will attract newer contributors who may have a fresher perspective on the project, and I think that's a good thing.

It is also not the Git project's responsibility to ensure that every platform is Rust-friendly.

Hopefully the platforms that we currently support but won't after this patch series have niche enough workloads that they do not need the absolute latest-and-greatest Git release at all times.

Yeah, I think that this is the most interesting part of the discussion here. I am not knowledgeable enough about Rust's release cadence and platform compatibility to have an opinion here. But I trust brian's judgement ;-).

Quoted 3 lines

I am far from a Rust expert, but I think that a more modern, memory-safe language will attract newer contributors who may have a fresher perspective on the project, and I think that's a good thing.

Aren't they likely to contribute to gitoxide? There, they get a clean slate without having to deal with the least-fun part (bidings).

Quoted 2 lines

It is also not the Git project's responsibility to ensure that every platform is Rust-friendly.

That's true, of course. And nobody is entitled to indefinie updates, but on the other hand, there's still some implicit contract with users. I really don't think git would have the adoption it does today if it had adopted a Rust-like language in the same state Rust is now from the start.

(In exactly the same way, git doesn't gratuitously break compatibility every release either. Can it? Yes, and git can change the platforms it runs on, but it's something to be taken seriously.)

Quoted 3 lines

Hopefully the platforms that we currently support but won't after this patch series have niche enough workloads that they do not need the absolute latest-and-greatest Git release at all times.

I mention this in my other email, but it's not just about ancient platforms. It's also about new ones, or ones where Rust supports them poorly despite them being relevant.

Hi,

On Tue, Jul 22, 2025 at 9:03 AM Sam James <[email protected]> wrote:

First of all, thanks to all the Gentoo folks for chiming in and providing specifics about platforms and their state.

Quoted 5 lines

Quoted 4 lines

I am far from a Rust expert, but I think that a more modern, memory-safe language will attract newer contributors who may have a fresher perspective on the project, and I think that's a good thing.

Quoted 2 lines

Aren't they likely to contribute to gitoxide? There, they get a clean slate without having to deal with the least-fun part (bidings).

I'm sure some are. But clearly there are others where the draw is improving git itself because of its installed base; in fact, we need look no further than this exact series we are commenting on to find proof of that -- one such new contributor submitted patches to use Rust in git, and found a significant speedup while doing so.

Further, there's considerable interest from existing git developers to use Rust in git as well; last year at the Git contributor summit, usage of Rust in git was not only one of the topics of discussion, it was the top voted topic (meaning, the topic that the greatest number of git contributors wanted to discuss).

Quoted 9 lines

Quoted 2 lines

It is also not the Git project's responsibility to ensure that every platform is Rust-friendly.

That's true, of course. And nobody is entitled to indefinie updates, but on the other hand, there's still some implicit contract with users. I really don't think git would have the adoption it does today if it had adopted a Rust-like language in the same state Rust is now from the start.

Quoted 3 lines

(In exactly the same way, git doesn't gratuitously break compatibility every release either. Can it? Yes, and git can change the platforms it runs on, but it's something to be taken seriously.)

This feels kind of close to a false dichotomy between breaking compatibility every release and indefinite update entitlements. There is certainly some middle ground: discussing reducing the breadth of platform support in order to gain other benefits, then gathering feedback, making a plan, and announcing the upcoming change, etc.

And we're already pretty deep into it. Concerns about losing out on some platforms have repeatedly slowed us down from adopting Rust years ago. Yet, the desire for Rust adoption keeps coming up anyway; see the threads starting at

* https://lore.kernel.org/git/[email protected]/
* https://lore.kernel.org/git/Zu2D%[email protected]/
* https://lore.kernel.org/git/[email protected]/
(search for "Rust")
* https://lore.kernel.org/git/[email protected]/

The discussion has also been picked up and reported outside the Git mailing list, e.g. https://lwn.net/Articles/998115/.

And so, in addition to the optional contrib/libgit-rs and contrib/libgit-sys Rust components that have already been merged into git, and a new build system added in part to make it easier to adopt Rust, we now have the first patch series that proposes a hard dependency on Rust.

Further, I'd like to comment a bit on the support of our users from another angle. We're also responsible for security for our users

and
feel Rust would help (see e.g.
https://litchipi.github.io/infosec/2023/01/24/git-code-audit-viewed-as-rust-programmer.html
and https://github.com/bk2204/git/commit/fbeb1180c7473635a964daed2da642c53487782d).
We're responsible for performance of Git for our users, and feel Rust would help (see the email that started this thread,
https://lore.kernel.org/git/CABPp-BFOmwV-xBtjvtenb6RFz9wx2VWVpTeho0k=D8wsCCVwqQ@mail.gmail.com/,
and brian's notes about [CPU multi-]threading elsewhere in this email thread we are in). And there are other benefits from using Rust that

we believe would benefit our users. Thus, it's not just a question of responsibility to our users, because such a responsibility pulls us in different directions regarding usage of Rust. So we need to figure out how to weigh the needs of our different users. For many of us, and forgive the geeky comparison, we'll probably weigh those needs with something more akin to an L2 norm (most good for the most users) rather than an L-infinity norm (maximal difference in usability for a single user), which probably isn't to your liking.

Anyway, there's been lots of discussion already. We can certainly still discuss more about exactly how to announce, when to adopt Rust, whether we'll support an existing C-only version of git for a longer period of time than normal, and even whether to continue to delay adopting Rust for a little longer. But my personal guess is that attempting to stop adoption of Rust is unlikely to win at this point.

Quoted 8 lines

Quoted 4 lines

Hopefully the platforms that we currently support but won't after this patch series have niche enough workloads that they do not need the absolute latest-and-greatest Git release at all times.

I mention this in my other email, but it's not just about ancient platforms. It's also about new ones, or ones where Rust supports them poorly despite them being relevant.

This feels like you're trying to push the decision for a given platform to be a dichotomy between latest-and-greatest-Git or no-version-of-Git-at-all, despite the fact that Taylor suggested an alternative and you even quoted him. Can you comment on that alternative? Why would using the last C-only version of Git[1] until gccrs bridges the gap be a problem for these platforms?

Thanks,
Elijah

[1] Well, C-only other than optional Rust components like contrib/libgit-rs and contrib/libgit-sys that have already been released.

Quoted 2 lines

Further, I'd like to comment a bit on the support of our users from another angle. We're also responsible for security for our users

Supply-chain issues become more of a problem with Rust if we end up making heavy use of crates. A policy moderating their use is something we should talk about.

This series accelerates xdiff by 5-19%.

It also introduces Rust as a hard dependency.

…and it doesn’t yet pass a couple of the github workflows; hints from Windows experts, and opinions on ambiguous primitives would be appreciated (see below).

This is just the beginning of many patches that I have to convert portions of, maybe eventually all of, xdiff to Rust. While working on that conversion, I found several ways to clarify the code, along with some optimizations.

Quoted 4 lines · Ezekiel Newren via GitGitGadget

This series accelerates xdiff by 5-19%.

It also introduces Rust as a hard dependency.

Quoted 8 lines

…and it doesn’t yet pass a couple of the github workflows; hints from Windows experts, and opinions on ambiguous primitives would be appreciated (see below).

This is just the beginning of many patches that I have to convert portions of, maybe eventually all of, xdiff to Rust. While working on that conversion, I found several ways to clarify the code, along with some optimizations.

Just a quick heads-up: We (as in Vim/Neovim) have been using gits xdiff library for use in Vim and Neovim.

Is the plan to get rid of xdiffs C source completely and replace it by a Rust implementation?

Christian Brabandt <[email protected]> writes:

Quoted 5 lines

Just a quick heads-up: We (as in Vim/Neovim) have been using gits xdiff library for use in Vim and Neovim.

Is the plan to get rid of xdiffs C source completely and replace it by a Rust implementation?

As far as I know, there is no such plan that is widely agreed upon (yet).

The discussion starter thread you are looking at only introduces a new code path that uses a different line hash function written in Rust when whitespace munging search is not enabled, and everything else still is written in C, but since it is just a discussion starter so far.

I would personally have liked the effort to start with xmerge code, not xdiff machinery, for various reasons, but that may be just me ;-)

This series accelerates xdiff by 5-19%.

It also introduces Rust as a hard dependency.

…and it doesn’t yet pass a couple of the github workflows; hints from Windows experts, and opinions on ambiguous primitives would be appreciated (see below).

This is just the beginning of many patches that I have to convert portions of, maybe eventually all of, xdiff to Rust. While working on that conversion, I found several ways to clarify the code, along with some optimizations.

So...

This obviously raises the question of whether we are ready to accept a hard dependency on Rust. Previous discussions on the mailing list and at Git Merge 2024 have not answered that question. If not now, will we be willing to accept such a hard dependency later? And what route do we want to take to get there?

About the optimizations in this series:

1. xdiff currently uses DJB2a for hashing (even though it is not explicitly named as such). This is an older hashing algorithm, and modern alternatives are superior. I chose xxhash because it’s faster, more collision resistant, and designed to be a standard. Other hash algorithms like aHash, MurMurHash, SipHash, and Fnv1a were considered, but my local testing made me feel like xxhash was the best choice for usage in xdiff.

2. In support of switching to xxhash, parsing and hashing were split into separate steps. And it turns out that memchr() is faster for parsing than character-by-character iteration.

About the workflow builds/tests that aren’t working with this series:

1. Windows fails to build. I don’t know which rust toolchain is even correct for this or if multiple are needed. Example failed build: https://github.com/git/git/actions/runs/16353209191

2. I386/ubuntu:focal will build, but fails the tests. The kernel reports the bitness as 64 despite the container being 32. I believe the issue is that C uses ambiguous primitives (which differ in size between platforms). The new code should use unambiguous primitives from Rust (u32, u64, etc.) rather than perpetuating ambiguous primitive types. Since the current xdiff API hardcodes the ambiguous types, though, those places will need to be migrated to unambiguous primitives. Much of the C code needs a slight refactor to be compatible with the Rust FFI and usually requires converting ambiguous to unambiguous types. What does this community think of this approach?

My brother (Elijah, cc’ed) has been guiding and reviewing my work here.

Ezekiel Newren (7):
xdiff: introduce rust
xdiff/xprepare: remove superfluous forward declarations
xdiff: delete unnecessary fields from xrecord_t and xdfile_t
xdiff: make fields of xrecord_t Rust friendly
xdiff: separate parsing lines from hashing them
xdiff: conditionally use Rust's implementation of xxhash
github_workflows: install rust

  .github/workflows/main.yml |   1 +
  .gitignore                 |   1 +
  Makefile                   |  60 +++++++---
  build_rust.sh              |  59 ++++++++++
  ci/install-dependencies.sh |  14 +--
  ci/install-rust.sh         |  33 ++++++
  ci/lib.sh                  |   8 ++
  ci/make-test-artifacts.sh  |   7 ++
  ci/run-build-and-tests.sh  |  10 ++
  git-compat-util.h          |  17 +++
  meson.build                |  40 +++++--
  rust/Cargo.lock            |  21 ++++
  rust/Cargo.toml            |   6 +
  rust/interop/Cargo.toml    |  14 +++
  rust/interop/src/lib.rs    |   0
  rust/xdiff/Cargo.toml      |  16 +++
  rust/xdiff/src/lib.rs      |   7 ++
  xdiff/xdiffi.c             |   8 +-
  xdiff/xemit.c              |   2 +-
  xdiff/xmerge.c             |  14 +--
  xdiff/xpatience.c          |   2 +-
  xdiff/xprepare.c           | 226 ++++++++++++++++++-------------------
  xdiff/xtypes.h             |   9 +-
  xdiff/xutils.c             |   4 +-
  24 files changed, 414 insertions(+), 165 deletions(-)

create mode 100755 build_rust.sh
create mode 100644 ci/install-rust.sh
create mode 100644 rust/Cargo.lock
create mode 100644 rust/Cargo.toml
create mode 100644 rust/interop/Cargo.toml
create mode 100644 rust/interop/src/lib.rs
create mode 100644 rust/xdiff/Cargo.toml
create mode 100644 rust/xdiff/src/lib.rs

base-commit: 16bd9f20a403117f2e0d9bcda6c6e621d3763e77
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1980%2Fezekielnewren%2Fxdiff_rust_speedup-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1980/ezekielnewren/xdiff_rust_speedup-v1
Pull-Request: https://github.com/git/git/pull/1980

Hi Ezekiel

Thanks for working on this

Quoted 10 lines · Ezekiel Newren via GitGitGadget

So...

This obviously raises the question of whether we are ready to accept a hard
dependency on Rust. Previous discussions on the mailing list and at Git Merge 2024 have not answered that question. If not now, will we be willing
to accept such a hard dependency later? And what route do we want to take to
get there?

As far as git goes I think introducing a hard dependency on rust is fine. It is widely supported, the only issue I'm aware of is the lack of support on NonStop and I don't think it is reasonable for such a minority platform to hold the rest of the project to ransom. There is a question about the other users of the xdiff code though. libgit2 carries a copy as do other projects like neovim. I've cc'd the libgit2 maintainer and posted a link to this thread in neovim github [1]

Quoted 17 lines

Hi Ezekiel

Thanks for working on this

Quoted 10 lines · Ezekiel Newren via GitGitGadget

So...

This obviously raises the question of whether we are ready to accept a hard
dependency on Rust. Previous discussions on the mailing list and at Git Merge 2024 have not answered that question. If not now, will we be willing
to accept such a hard dependency later? And what route do we want to take to
get there?

Quoted 7 lines

As far as git goes I think introducing a hard dependency on rust is fine. It is widely supported, the only issue I'm aware of is the lack of support on NonStop and I don't think it is reasonable for such a minority platform to hold the rest of the project to ransom. There is a question about the other users of the xdiff code though. libgit2 carries a copy as do other projects like neovim. I've cc'd the libgit2 maintainer and posted a link to this thread in neovim github [1]

A hard dependency on rust for Gentoo amd64 would potentially require building https://github.com/thepowersgang/mrustc followed by building 13 and counting versions of rustc in order to get to the latest version. What is the minimum supported version in this series, by the way?

bin packages for rust do exist but not everyone wants to use non-distro provided binaries, sometimes for auditability reasons.

For Gentoo HPPA, Alpha, m68k it will simply mean the removal (or end of life and staying forever on 2.50, perhaps) of Git. There is no rust compiler there.

Even s390 support for rust is limited to a precompiled version not everyone is willing to use.

GCC-rs will probably fix this general issue.

Signature · 1 line

Eli Schwartz

[2025-07-18 17:25:01-0400] Eli Schwartz:

Quoted 36 lines · Phillip Wood

Quoted 17 lines

Hi Ezekiel

Thanks for working on this

Quoted 10 lines · Ezekiel Newren via GitGitGadget

So...

This obviously raises the question of whether we are ready to accept a hard
dependency on Rust. Previous discussions on the mailing list and at Git Merge 2024 have not answered that question. If not now, will we be willing
to accept such a hard dependency later? And what route do we want to take to
get there?

Quoted 7 lines

As far as git goes I think introducing a hard dependency on rust is fine. It is widely supported, the only issue I'm aware of is the lack of support on NonStop and I don't think it is reasonable for such a minority platform to hold the rest of the project to ransom. There is a question about the other users of the xdiff code though. libgit2 carries a copy as do other projects like neovim. I've cc'd the libgit2 maintainer and posted a link to this thread in neovim github [1]

A hard dependency on rust for Gentoo amd64 would potentially require building https://github.com/thepowersgang/mrustc followed by building 13 and counting versions of rustc in order to get to the latest version. What is the minimum supported version in this series, by the way?

bin packages for rust do exist but not everyone wants to use non-distro provided binaries, sometimes for auditability reasons.

Quoted 6 lines

For Gentoo HPPA, Alpha, m68k it will simply mean the removal (or end of life and staying forever on 2.50, perhaps) of Git. There is no rust compiler there.

Even s390 support for rust is limited to a precompiled version not everyone is willing to use.

Also in other distro concerns, if it trickles down to libgit2, extra care should be taken to avoid creating circular dependencies due to cargo depending on libgit2 (via git2 crate).

For example with making sure it can reasonably be built via meson's Rust support rather than through cargo.

Quoted 17 lines · Phillip Wood

Hi Ezekiel

Thanks for working on this

Quoted 10 lines · Ezekiel Newren via GitGitGadget

So...

This obviously raises the question of whether we are ready to accept a hard
dependency on Rust. Previous discussions on the mailing list and at Git Merge 2024 have not answered that question. If not now, will we be willing
to accept such a hard dependency later? And what route do we want to take to
get there?

Quoted 7 lines

As far as git goes I think introducing a hard dependency on rust is fine. It is widely supported, the only issue I'm aware of is the lack of support on NonStop and I don't think it is reasonable for such a minority platform to hold the rest of the project to ransom. There is a question about the other users of the xdiff code though. libgit2 carries a copy as do other projects like neovim. I've cc'd the libgit2 maintainer and posted a link to this thread in neovim github [1]

A hard dependency on rust for Gentoo amd64 would potentially require building https://github.com/thepowersgang/mrustc followed by building 13 and counting versions of rustc in order to get to the latest version. What is the minimum supported version in this series, by the way?

bin packages for rust do exist but not everyone wants to use non-distro provided binaries, sometimes for auditability reasons.

For Gentoo HPPA, Alpha, m68k it will simply mean the removal (or end of life and staying forever on 2.50, perhaps) of Git. There is no rust compiler there.

Even s390 support for rust is limited to a precompiled version not everyone is willing to use.

GCC-rs will probably fix this general issue.

Quoted 8 lines · Eli Schwartz

For Gentoo HPPA, Alpha, m68k it will simply mean the removal (or end of life and staying forever on 2.50, perhaps) of Git. There is no rust compiler there.

Even s390 support for rust is limited to a precompiled version not everyone is willing to use.

GCC-rs will probably fix this general issue.

Hm. It would be nice to assemble a list of common or semi-common distributions that do not have proper support for Rust for all or at least some platforms. Should we maybe consider reaching out to other distros (e.g. Debian, Fedora, BSDs) before we commit to any change that has an outsized impact on the larger ecosystem?

I would really love to start adopting Rust, and if it's only going to be architectures that are extremely niche I'm probably fine with that. But if there are many small systems that are impacted by such a change we might have to reconsider.

Meh :/

There's a few issues from our perspective:

* Old platforms which don't have LLVM can't yet have Rust either, as rustc is based on LLVM.

These need gccrs to be unblocked. I can understand not caring too much about these, though it is unfortunate, because I think if git hadn't supported many platforms to begin with, I doubt it'd have the adoption it does today.

(There is another effort which seeks to take rustc and bolt on libgccjit as a replacement backend, but that isn't feasible for use yet either.)

* rustc doesn't have LTS releases or the like.

The only supported release is the latest one. Upgrading to the latest release often means we have to deal with new portability problems but we can't not upgrade because:
a) some software will start to require bleeding-edge Rust immediately, and
b) it means we're missing out on bug fixes (miscompilations are serious)

* Crate creep

Rust projects tend to end up having a huge list of crates that they pull-in which makes us worried about something nasty creeping in, but there's also popular crates with serious portability problems like the 'ring' crate for TLS.

It would be great to know about the general timelines of these alternative implementations.

Quoted 2 lines · Patrick Steinhardt

It would be great to know about the general timelines of these alternative implementations.

We still think we'll be able to compile libcore before the end of the summer, we've made great progress and few items are left. But keep in mind we're targeting an older version of rust (1.49) and libcore is smaller than the standard library. We still have a lot of testing to do and we expect many bugs.

The next targeted version will probably be rust 1.78 as we want to keep up with rust for linux. This shouldn't be too long as most of the features are coming from either standard library modifications or nightly features we already had to support for 1.49.

We expect to be able to compile some 1.49 code correctly next year at best.

I would like to bring to your attention rustc_codegen_gcc which adds a gcc backend to the rustc frontend, although not a full gcc compiler it could help supporting some architectures that are currently not supported by llvm.

And I expect that 1.78 will be another significant effort that won't land before the year after?

For my own understanding: is this something that the Git project would have to support or something that the distributor needs to set up?

This series accelerates xdiff by 5-19%.

Quoted 1 line · Ezekiel Newren via GitGitGadget

This series accelerates xdiff by 5-19%.

;-)

Do we know how much of that can be attributed to the hash algorithm difference, and how much for languages?

The earlier parts of the series to trim unused code and refactor look to me that they are good changes regardless of whether we introduce a different hash algorithm, and/or we use an implementation of that different hash algorithm written in Rust. IOW, even if neither of these two happens, I would think that the earlier parts are independently good pieces.

Thanks for starting this effort. And thanks Elijah for helping.

And in case nobody has said this yet, welcome to the Git development community.

Quoted 1 line · Ezekiel Newren via GitGitGadget

This series accelerates xdiff by 5-19%.

;-)

Do we know how much of that can be attributed to the hash algorithm difference, and how much for languages?

... by 15%. Just over half of that seems to come from hoisting the check for whitespace flags in xdl_hash_record() out of the loop in xdl_prepare_ctx() and the rest comes from the change in hash function.

As I understand it the hash is implemented using SIMD compiler intrinsics and the rust implementation is basically a copy of the C code in libxxhash. I wonder how well xxhash performs compared to our existing hash on platforms without an optimized implementation.

1. Windows fails to build. I don’t know which rust toolchain is even correct for this or if multiple are needed. Example failed build:

   https://github.com/git/git/actions/runs/16353209191

Hi Ezekiel,
pleasure to make your acquaintance!

Quoted 3 lines · Ezekiel Newren via GitGitGadget

1. Windows fails to build. I don’t know which rust toolchain is even correct for this or if multiple are needed. Example failed build:

   https://github.com/git/git/actions/runs/16353209191

There are a couple of problems, not just one. Here are the patches that I would like to ask you to take custody of (for your convenience, I have pushed them to https://github.com/dscho/git as the xdiff_rust_speedup branch). Please find them below. They _just_ fix the build, but the tests with win+Meson still fail (and as "win+Meson test" jobs keep the logs of the failed tests a well-guarded secret, due to time constraints I have to stop looking into this for now).

Thank you for working on this,
Johannes

-- snipsnap --
From 72c50ee3f9df5ccfe48bf6f44b2c6bba05a680bf Mon Sep 17 00:00:00 2001
From: Johannes Schindelin <[email protected]>
Date: Sat, 19 Jul 2025 21:24:07 +0200
Subject: [PATCH 1/3] Do support Windows again after requiring Rust

By default, Rust wants to build MS Visual C-compatible libraries on Windows, because that is _the_ native C compiler.

Git is historically lacking in its MSVC support, and the official Git for Windows versions are built using GCC instead. As a consequence, a (subset of a) GCC toolchain is installed as part of the windows-build job of every CI build.

Naturally, this requires adjustments in how Rust is called, most importantly it requires installing support for a GCC-compatible build target.

Let's make the necessary adjustment both in the CI-specific code that installs Rust as well as in the Windows-specific configuration in config.mak.uname.

Signed-off-by Johannes Schindelin <[email protected]>

---
ci/install-rust.sh | 3 +++
config.mak.uname | 9 +++++++++

2 files changed, 12 insertions(+)

ci/install-rust.sh +0 -0

index 141ceddb17cfe..c22baa629ceb7 100644
@@ -28,6 +28,9 @@ if [ "$BITNESS" = "32" ]; then
  $CARGO_HOME/bin/rustup default --force-non-host $RUST_VERSION || exit $?

else
$CARGO_HOME/bin/rustup default $RUST_VERSION || exit $?

ci/install-rust.sh +3 -0

+  if [ "$CI_OS_NAME" = "windows" ]; then
+    $CARGO_HOME/bin/rustup target add x86_64-pc-windows-gnu || exit $?
+  fi

fi

. $CARGO_HOME/env

config.mak.uname +0 -0

index 3e26bb074a4b5..fbe7cebf40edd 100644