From: Toon Claes <toon@iotcl•com>
To: Patrick Steinhardt <ps@pks•im>, git@vger•kernel.org
Cc: "brian m. carlson" <sandals@crustytoothpaste•net>,
Ezekiel Newren <ezekielnewren@gmail•com>,
Junio C Hamano <gitster@pobox•com>
Subject: Re: [PATCH v2 5/5] rust: generate bindings via cbindgen
Date: Fri, 24 Oct 2025 16:01:07 +0200 [thread overview]
Message-ID: <87v7k4pffg.fsf@iotcl.com> (raw)
In-Reply-To: <20251024-b4-pks-rust-cbindgen-v2-5-4b4bd4f18490@pks.im>
Patrick Steinhardt <ps@pks•im> writes:
> [snip]
>
> diff --git a/meson.build b/meson.build
> index 308798e861..b4acc417ad 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -523,6 +523,7 @@ libgit_sources = [
> 'usage.c',
> 'userdiff.c',
> 'utf8.c',
> + 'varint.c',
> 'version.c',
> 'versioncmp.c',
> 'walker.c',
> @@ -1704,7 +1705,9 @@ version_def_h = custom_target(
> libgit_sources += version_def_h
>
> cargo = find_program('cargo', dirs: program_path, native: true, required: get_option('rust'))
> -rust_option = get_option('rust').disable_auto_if(not cargo.found())
> +cbindgen = find_program('cbindgen', dirs: program_path, native: true, required: get_option('rust'))
> +
> +rust_option = get_option('rust').disable_auto_if(not cargo.found() or not cbindgen.found())
This means to compile with Rust we not only need cargo, but also
cbindgen. As we ideally want to have a broad platform support, would
adding another dependency (i.e. `cbindgen`) narrow the platforms we'd
eventually support? Or can we consider platforms supporting Rust also
have cbindgen?
> [snip]
>
> diff --git a/varint.c b/varint.c
> index 03cd54416b..1ed738a756 100644
> --- a/varint.c
> +++ b/varint.c
> @@ -1,6 +1,14 @@
> #include "git-compat-util.h"
> #include "varint.h"
>
> +/*
> + * When building with Rust we don't compile the C code, but we only verify
> + * whether the function signatures of our C bindings match the ones we have
> + * declared in "varint.h".
> + */
> +#ifdef WITH_RUST
> +# include "c-bindings.h"
So when we rewrite more subsystems into Rust, this will include
definitions from all those subsystems into this compilation unit.
If one subsystem with a Rust alternative implementation includes the
header from another subsystem with a Rust-alternative implementation,
the function signatures are checked twice, and errors are surfaced
twice. I guess this is a tradeoff we can accept for now, because:
* We only have one subsystem in Rust now.
* The approach in this patch simplifies the build setup.
We might revisit that at some point, but I can agree this is the most
sensical approach for now.
--
Cheers,
Toon
next prev parent reply other threads:[~2025-10-24 14:01 UTC|newest]
Thread overview: 38+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-10-23 7:17 [PATCH 0/3] rust: generate bindings via cbindgen Patrick Steinhardt
2025-10-23 7:17 ` [PATCH 1/3] ci: use Debian instead of deprecated i386/ubuntu Patrick Steinhardt
2025-10-23 17:56 ` Junio C Hamano
2025-10-24 6:36 ` Patrick Steinhardt
2025-10-23 7:17 ` [PATCH 2/3] meson: rename Rust library target Patrick Steinhardt
2025-10-23 7:17 ` [PATCH 3/3] rust: generate bindings via cbindgen Patrick Steinhardt
2025-10-23 18:00 ` Ezekiel Newren
2025-10-24 6:37 ` Patrick Steinhardt
2025-10-27 20:35 ` Ezekiel Newren
2025-10-27 21:14 ` brian m. carlson
2025-10-28 4:15 ` Junio C Hamano
2025-10-28 19:11 ` Ezekiel Newren
2025-10-30 9:50 ` Patrick Steinhardt
2025-10-30 9:50 ` Patrick Steinhardt
2025-10-30 21:40 ` brian m. carlson
2025-10-30 21:50 ` Junio C Hamano
2025-10-30 23:38 ` brian m. carlson
2025-10-31 6:05 ` Patrick Steinhardt
2025-10-30 9:50 ` Patrick Steinhardt
2025-10-31 23:36 ` Ezekiel Newren
2025-10-23 21:42 ` Junio C Hamano
2025-10-23 22:01 ` Junio C Hamano
2025-10-23 22:37 ` Junio C Hamano
2025-10-24 6:36 ` Patrick Steinhardt
2025-10-24 9:51 ` [PATCH v2 0/5] " Patrick Steinhardt
2025-10-24 9:51 ` [PATCH v2 1/5] gitlab-ci: reorder Linux job matrix to match GitHub's order Patrick Steinhardt
2025-10-28 19:14 ` Ezekiel Newren
2025-10-24 9:51 ` [PATCH v2 2/5] gitlab-ci: backfill missing Linux jobs Patrick Steinhardt
2025-10-28 19:15 ` Ezekiel Newren
2025-10-24 9:51 ` [PATCH v2 3/5] ci: use Debian instead of deprecated i386/ubuntu Patrick Steinhardt
2025-10-28 19:17 ` Ezekiel Newren
2025-10-30 9:50 ` Patrick Steinhardt
2025-10-24 9:51 ` [PATCH v2 4/5] meson: rename Rust library target Patrick Steinhardt
2025-10-24 9:51 ` [PATCH v2 5/5] rust: generate bindings via cbindgen Patrick Steinhardt
2025-10-24 14:01 ` Toon Claes [this message]
2025-10-30 9:51 ` Patrick Steinhardt
2025-10-28 19:37 ` [PATCH v2 0/5] " Ezekiel Newren
2025-10-30 9:50 ` Patrick Steinhardt
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87v7k4pffg.fsf@iotcl.com \
--to=toon@iotcl$(echo .)com \
--cc=ezekielnewren@gmail$(echo .)com \
--cc=git@vger$(echo .)kernel.org \
--cc=gitster@pobox$(echo .)com \
--cc=ps@pks$(echo .)im \
--cc=sandals@crustytoothpaste$(echo .)net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox