git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Patrick Steinhardt <ps@pks.im>
To: "brian m. carlson" <sandals@crustytoothpaste.net>,
	Ezekiel Newren <ezekielnewren@gmail.com>,
	git@vger.kernel.org
Subject: Re: [PATCH 3/3] rust: generate bindings via cbindgen
Date: Thu, 30 Oct 2025 10:50:55 +0100	[thread overview]
Message-ID: <aQM0_6uRZcQYfO8R@pks.im> (raw)
In-Reply-To: <aP_gy-Rj8MI7zAWd@fruit.crustytoothpaste.net>

On Mon, Oct 27, 2025 at 09:14:51PM +0000, brian m. carlson wrote:
> On 2025-10-27 at 20:35:59, Ezekiel Newren wrote:
> > On Fri, Oct 24, 2025 at 12:37 AM Patrick Steinhardt <ps@pks.im> wrote:
> > > > cbindgen is a Rust crate and it should be specified in the Cargo.toml
> > > > under [build-dependencies] block.
> > >
> > > What is the benefit for us? The generated code is not a dependency of
> > > the Rust code, and neither do we use it via "build.rs". And if we use
> > > cbindgen via "Cargo.toml" we'd be forced to build it first, which slows
> > > down our CI jobs.
> > >
> > > Please let me know in case I miss any reasons to have it in our build
> > > dependencies instead.
> > 
> > You're targeting a very old version of Rust (1.49). I'm not even sure
> > that cbindgen will work with a version that old, but if it does then
> > we should use it in build.rs to make sure we're not using any features
> > of cbindgen that aren't available until later versions. If we use
> > cbindgen that is packaged with the platform then we can't precisely
> > control which version of cbindgen is being used. This is a matter of
> > reproducibility. There may be platforms that can compile Rust, but
> > can't generate C header files via cbindgen because cbindgen hard codes
> > that a certain minimum Rust version is required in its own Cargo.toml
> > file.
> 
> Yes, I agree with this.  Not all systems have cbindgen and it's not
> guaranteed that the system's cbindgen will work with the version of Rust
> that you want to target or that's being used to compile.

Okay. In that case the question to me is how to drive cbindgen from our
Makefile and from Meson if it's going to be invoked via "build.rs". It
doesn't make much sense from my PoV to make generation of the C headers
depend on building the complete Rust library. Doubly so because we'd now
have a chicken-and-egg problem:

  1. To build libgit.a we need to have the C interop header.

  2. To build the C interop header we need to build the Rust library.

  3. The Rust library depends on libgit.a.

So am I missing anything obvious here for how to declare cbindgen in our
"Cargo.toml" file and invoke it directly from our other build systems?

Thanks!

Patrick

  parent reply	other threads:[~2025-10-30  9:51 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 [this message]
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
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=aQM0_6uRZcQYfO8R@pks.im \
    --to=ps@pks.im \
    --cc=ezekielnewren@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=sandals@crustytoothpaste.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
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;
as well as URLs for NNTP newsgroup(s).